Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assembler label problem and work on MPIR #330

Closed
GitMensch opened this issue Jan 25, 2019 · 44 comments
Closed

assembler label problem and work on MPIR #330

GitMensch opened this issue Jan 25, 2019 · 44 comments

Comments

@GitMensch
Copy link
Contributor

The following assembler constructs don't compile (generated by MPIR [configure and make otherwise works] but most of the assembler sources look that way so all sub-directories fail to be generated):

	.text
	.align	4, 0x90
	.globl	___gmpn_add_err1_n
	.type	___gmpn_add_err1_n,@function
___gmpn_add_err1_n:

	mov	8(%rsp), %rax    

	push	%rbx
	push	%rbp
	push	%r12

	xor %rbx, %rbx              
	xor %rbp, %rbp
	lea	(%rdi,%r9,8), %rdi     
	lea	(%rsi,%r9,8), %rsi
	lea	(%rdx,%r9,8), %rdx

Error tmp-add_err1_n.s(74): Label '___gmpn_add_err1_n' already exists.
Error tmp-add_err1_n.s(76): Constant value expected
Error tmp-add_err1_n.s(78): Unknown token sequence
Error tmp-add_err1_n.s(79): Unknown token sequence
Error tmp-add_err1_n.s(80): Unknown token sequence
Error tmp-add_err1_n.s(82): Unknown token sequence
Error tmp-add_err1_n.s(83): Unknown token sequence
Error tmp-add_err1_n.s(84): Constant value expected
Error tmp-add_err1_n.s(85): Constant value expected
Error tmp-add_err1_n.s(86): Constant value expected

Complete file (don't ask me why there are so much empty lines in it...):
tmp-add_err1_n.s

@LADSoft
Copy link
Owner

LADSoft commented Jan 25, 2019

ok there are four problems - I'm mostly writing this now so you will be aware of #4...

  1. .globl isn't working as expected - will fix
  2. uses gnu assembler syntax (register names and operand order) - maybe I will add a command line switch to accept GNU syntax?
  3. this is 64 bit code - maybe I will add a command line switch to select that as well (normal usage for this assembler is to embed a directive in the source file)
  4. because it is 64-bit code and OCC is a 32-bit compiler, when we get the output compiling it still won't run...

@GitMensch
Copy link
Contributor Author

GitMensch commented Jan 25, 2019

  1. good
  2. Yes, A command line switch seems to be appropriate, I suggest an option duo like --gas GNU assembler mode (extended AT&T) and --intel Intel assembler mode (default). If you add a switch here then I'd vote to be strict about the syntax - either intel or at&t (a switch from the later to the former should be possible per in-source per-file by using .intel_syntax "directive") but not allow a mixture like it is done now, if you go for "excellent" you may do a minimal parsing of the conflicting syntax and raise an error/warning "Syntax does not match the current assembler mode, assemble again using --xyz to switch".
  3. getting it compiling (with a warning) is good in any case and would allow coming back to this when 64-bit work is done some day
  4. it is likely that there's a configure switch for MPIR to force 32bit, I'll check once 1+2 are done

@LADSoft
Copy link
Owner

LADSoft commented Feb 4, 2019

have not yet gotten traction on this... maybe this week

@LADSoft
Copy link
Owner

LADSoft commented Feb 9, 2019

i'm starting to look at this... the .globl problem is probably due to the .type statement, so I guess a part of getting the end result we want has got to be to completely address #236 as well...

@GitMensch
Copy link
Contributor Author

completely address #236

8-|
... completely sounds marvelous (and like a lot of work)

As there's a lot of code out there that actually is using gas I think it is worth the work.

@LADSoft
Copy link
Owner

LADSoft commented Feb 19, 2019

sorry for the delay on this and the other things you've brought up recently... I've been dealing with a lot of unexpected compiler bugs that surfaced when I replaced pointers with 'unique_ptr'. To get the unique_ptr to work I had to fix a few bugs and it kind of snowballed on me... fix for one bug broke three other things sigh. I was trying to work it as a background task on another branch but the code diverged so much i decided to just get it finished.

As of tonite I have the compiler compiling itself again well enough that it can compile other programs... but there is a problem with one of the more problematic programs in the test suite just hanging when I run it. I Hope to have any remaining problems worked out within the next few days and get back to working on the issues again.

@GitMensch
Copy link
Contributor Author

GitMensch commented Feb 19, 2019 via email

@LADSoft
Copy link
Owner

LADSoft commented Feb 19, 2019

thanks!

FWIW now I've apparently found some arcane bug in mingw64 that doesn't exist in any of the other compilers, including orange C. Guess I will work on finding a workaround tomorrow...

@chuggafan
Copy link
Contributor

mingw64 bug? That might be because you don't have the latest compiler version (GCC 7.2 or 8 should be available)? If it's up to date and still buggy, I recommend reporting it because it should be just a straight GCC ontop of an emulation layer...

@LADSoft
Copy link
Owner

LADSoft commented Feb 20, 2019

thanks for the tip. That helped. 8.10 seems to fix it; Previously I had 7.30.

@LADSoft
Copy link
Owner

LADSoft commented Feb 20, 2019

i'm gonna backtrack on that. Seems to work with the i686 builds of mingw64 but not with the x86_64 builds. Unfortunately we need the latter for appveyor... so I guess there has to be a workaround at minimum but if I can get that far I'll also report it.

@LADSoft
Copy link
Owner

LADSoft commented Feb 21, 2019

mea culpa, mingw works fine. There was a longstanding problem that just became uncovered for reasons unknown. If orangec was compiled with a 64 bit compiler it then wouldn't emit exception information properly. Will release a fix for that soon...

@LADSoft
Copy link
Owner

LADSoft commented Apr 10, 2019

I haven't forgotten this... I'm going to work on #355 for a a while then I will come back to it :)

@LADSoft
Copy link
Owner

LADSoft commented Sep 14, 2019

@GitMensch question for you, in the opening post of this issue some source code was given that doesn't compile.

the GAS documentation says that instruction size should be postfixed on the instruction, e.g.

xorl $4,%eax

but the example doesn't use the size postfix values. So I'm wondering what the rules for omitting them are. If omitted does the size default to DWORD? Or is the size derived from the register sizes
the way it is done in the intel syntax?

@LADSoft
Copy link
Owner

LADSoft commented Sep 14, 2019

never mind I found it in the documentation 😄

@LADSoft
Copy link
Owner

LADSoft commented Sep 20, 2019

I'm going to check in a fix for this tonite or tomorrow; but it only addresses the mnemonic/operand problems and related issues. The problem of GAS-like directives will be fixed in relation to #236 (also in this milestone)

LADSoft added a commit that referenced this issue Sep 20, 2019
LADSoft added a commit that referenced this issue Sep 20, 2019
LADSoft added a commit that referenced this issue Sep 20, 2019
LADSoft added a commit that referenced this issue Sep 20, 2019
@GitMensch
Copy link
Contributor Author

Rechecked with the merge of assembler branch CI build, does not work:

	.text
	.align	4, 0x90
	.globl	___gmpn_add_err1_n
	.type	___gmpn_add_err1_n,@function
___gmpn_add_err1_n:

	mov	8(%rsp), %rax    

	push	%rbx
	push	%rbp
	push	%r12

	xor %rbx, %rbx              
	xor %rbp, %rbp
	lea	(%rdi,%r9,8), %rdi     
	lea	(%rsi,%r9,8), %rsi
	lea	(%rdx,%r9,8), %rdx

	test	$1, %r9            
	jnz	Lodd

Leven:	
	lea	-8(%r8,%r9,8), %r8   
	neg	%r9                
	jmp	Ltop

	.align	4, 0x90
Lodd:                           
	lea	-16(%r8,%r9,8), %r8     
	neg	%r9                   
	shr	$1, %rax            
	mov	(%rsi,%r9,8), %r12
	adc	(%rdx,%r9,8), %r12
	cmovc	8(%r8), %rbx           
	mov	%r12, (%rdi,%r9,8)
	setc	%al                 
	inc	%r9                   
	jz	Lend              

	.align	4, 0x90
Ltop:
       mov     (%rsi,%r9,8), %r12     
	shr     $1, %rax        
	adc     (%rdx,%r9,8), %r12
	mov     $0, %r11          
	mov     %r12, (%rdi,%r9,8)
	mov     $0, %r10          
	mov     8(%rsi,%r9,8), %r12    
	cmovc   (%r8), %r10        
	adc     8(%rdx,%r9,8), %r12
	cmovc   -8(%r8), %r11      
	setc    %al             
	add     %r10, %rbx          
	adc     $0, %rbp
	add     %r11, %rbx          
	mov     %r12, 8(%rdi,%r9,8)
	adc     $0, %rbp
	add     $2, %r9           
	lea     -16(%r8), %r8     
	jnz     Ltop          

Lend:	
	mov	%rbx, (%rcx)         
	mov	%rbp, 8(%rcx)

	pop	%r12
	pop	%rbp
	pop	%rbx
	ret
	

Resulting in

Error tmp-add_err1_n.s(6): Label '___gmpn_add_err1_n' already exists.
Error tmp-add_err1_n.s(8): Constant value expected
Error tmp-add_err1_n.s(10): Unknown token sequence
...

Or is this related to the GAS-like directives issue?

@LADSoft
Copy link
Owner

LADSoft commented Sep 21, 2019

this is a problem with not supporting the gas-like directives, but I'll reopen this until it actually compiles your code snippet...

@LADSoft LADSoft reopened this Sep 21, 2019
@LADSoft
Copy link
Owner

LADSoft commented Sep 21, 2019

Hi @GitMensch, i fixed this so it will assemble your example now.

There may also have been some confusion; you additionally need to specify the command line option --gas:

 Oasm --gas test.asm

This was listed in the command line help already but I've updated the .MD files as well now. Note that I had additionally addressed #249 in this milestone, which allows for 'long' names to be used in switches as we move forward.

@LADSoft
Copy link
Owner

LADSoft commented Sep 28, 2019

the problem is a file in mpn\generic.

looks like some problem with the configure script. from build_occ\config.h:

/* #undef HAVE_DOUBLE_IEEE_BIG_ENDIAN */
/* #undef HAVE_DOUBLE_IEEE_LITTLE_ENDIAN */
/* #undef HAVE_DOUBLE_IEEE_LITTLE_SWAPPED */

so ieee_double_extract isn't getting defined from gmp-impl.h. the build for VC seems to have the little endian one defined and I think OCC should too.

Will see if I can figure out why the config script fails in this regard but may need help...

@LADSoft
Copy link
Owner

LADSoft commented Sep 29, 2019

ok I think the problem may be that the configure scripts assume the object files are binary in nature and are getting thrown for a loop by the fact the the OCC object files are ascii.

meanwhile I tried #defining the little endian one in config.h but still couldn't get past this...

@LADSoft
Copy link
Owner

LADSoft commented Sep 29, 2019

i figured out what I was doing wrong in config.h and it at least compiles everything in the MPN directory now. Then I had to add 'AR=olib' and regenerate the configuration... right now it is failing on the library insert I think because some of the object files are being generated in what appears to be COFF format... I think that is possibly because the project uses YASM?

@LADSoft
Copy link
Owner

LADSoft commented Sep 29, 2019

there is some problem with occ... if I compile occ with MSVC it then compiles the mpn directory, but if I then compile it with itself I then get errors when compiling the mpn directory e.g. in gmp-impl.h. Will fix soon...

BTW if the HAVE_DOUBLE_IEEE_LITTLE_ENDIAN detection and similar things that parse a binary formatted object file is a big deal it looks like I have already written a 'binary' version of the object file handling, but it is probably out of date and needs to be vetted further before it can be used...

@GitMensch
Copy link
Contributor Author

there is some problem with occ... if I compile occ with MSVC it then compiles the mpn directory, but if I then compile it with itself I then get errors when compiling the mpn directory e.g. in gmp-impl.h. Will fix soon...

Sounds good. So I'll wait until I hear something again (but effectively I'll just do the steps above and when those work run omake check - things you could do yourself, too). If there's a working binary version then I think occ and oasm proved themself to be usable for a lot of other projects :-)

BTW if the HAVE_DOUBLE_IEEE_LITTLE_ENDIAN detection and similar things that parse a binary formatted object file is a big deal [...]

For now the effect should only be that MPIR will do "normal" double/float handling and no binary optimizations.

it looks like I have already written a 'binary' version of the object file handling, but it is probably out of date and needs to be vetted further before it can be used...

Interesting. I think it would be reasonable to handle this with a new issue. It will be interesting to see if/what the effect in case of necessary compile/link time (and object file's size) this has.

@LADSoft
Copy link
Owner

LADSoft commented Sep 29, 2019

ok so there is some problem with attribute((noreturn)) in versions compiled with itself but for now it seems you can get around it by making sure to autoconfigure against the version of the compiler you intend to use. Autoconfigure apparently detects it isn't going to work and causes the compile not to use it. I will fix it tomorrow...

MPIR won't compile if you don't have one of the HAVE_DOUBLE flags set. In that case it doesn't define a needed union, so the compile throws errors. I've been assuming we need to get one defined and the way to get one defined is to switch to a binary object file format and then the autoconfig will detect floating point.

I'm with you, the binary object format needs a new issue... FWIW I originally wrote that in hopes it will speed things up but didn't see much happening and then decided not to finish it out. That was back when the compiler had an independent version of the object file format handling though. Maybe with the longer builds we will see something... it should cut the files size by quite a bit because right now every literal byte of data is encoded as two ASCII characters...

That won't fix everything though, use of YASM pretty much dictates it won't link. And meanwhile 'omake check' is faililng, it doesn't know how to create ../libmpir.la.

Will pick it up tomorrow 😄

@GitMensch
Copy link
Contributor Author

GitMensch commented Sep 30, 2019

I think there's a "slightly" topic drift here ;-)

MPIR won't compile if you don't have one of the HAVE_DOUBLE flags set. In that case it doesn't define a needed union, so the compile throws errors.

If this is the case then it is a MPIR bug, which is possible as most (all other?) compilers have a binary format. The solution is not to have a binary object format but to create and apply a minimal patch to the MPIR code - and/or to check if https://github.com/wbhart/mpir already contains the fix - you can get artifacts for it from https://ci.appveyor.com/project/wbhart/mpir (ignore the bad version number 3.0.0 it is up-to-date)

@LADSoft
Copy link
Owner

LADSoft commented Oct 1, 2019

I fixed the noreturn problem. It was much deeper than I first suspected, I'm surprised the compiler worked as well as it did...

@LADSoft
Copy link
Owner

LADSoft commented Oct 1, 2019

@GitMensch -

the config scripts assume MSVC when it isn't one of the GCC variants, and want to use LIB.EXE. There is even a line in there that says something to the effect of ,FIXME, WE REALLY SHOULD LET THE USER SPECIFY THE LIBRARIAN.,

a more serious issue is that YASM creates object files the occ toolchain cannot process, so even if I run the librarian separately I get a bunch of errors about object files having a bad format.

edited...

I looked at the 'omake check' issue and it seems like it may need the build to succeed before it will work. Is that correct?

@GitMensch
Copy link
Contributor Author

GitMensch commented Oct 2, 2019 via email

@LADSoft
Copy link
Owner

LADSoft commented Oct 2, 2019

what I'm getting from configure is assembly language files that cannot be processed by oasm in either mode; hence YASM. But if we need YASM to create object files it just isn't going to work.

@LADSoft LADSoft closed this as completed Oct 2, 2019
@LADSoft LADSoft reopened this Oct 2, 2019
@LADSoft
Copy link
Owner

LADSoft commented Oct 3, 2019

I was thinking, if there is some way to get autoconfig to generate gas-compatible assembly language files from the existing stubs, then we wouldn't need YASM and we could also test the oasm GAS compatibility some more. But I haven't a clue as to whether it is even possible, or how to go about it if it is...

@GitMensch
Copy link
Contributor Author

GitMensch commented Oct 3, 2019 via email

@LADSoft
Copy link
Owner

LADSoft commented Oct 7, 2019

i asked and according to the author of MPIR it can't use GAS. yasm is a requirement at the moment as the MPIR code uses features specific to yasm. there have been proposals to rewrite the assembly code for GAS but noone has found the time to do it.

I've gone as far as I can with MPIR.

@LADSoft LADSoft closed this as completed Oct 9, 2019
@GitMensch
Copy link
Contributor Author

I think there is at least one thing to do here as there is one assembler related error with MPIR (and a bunch of asserts) [I don't expect the result to "work" any more because of the yasm+linking isuse].

Things to do: setup system (using current "MINGW64, then static lib") as noted above.

Shown assembler issues (just reran omake -k after the first time) - @LADSoft please reopen because of these:

Syntax errors:

Error tmp-divexact_1.s(107): Label '%' already exists.
Error tmp-modexact_1c_odd.s(113): Label '%' already exists.

Current assert (3 times):

Access Violation:(C:\dev\OrangeC\bin\oasm.exe)
CS:EIP 0023:00506A3D  SS:ESP 002B:0019E58C
EAX: 0000001C  EBX: 00000000  ECX: 0000001C  EDX: 00000000  flags: 00010202
EBP: 005B172E  ESI: 00000018  EDI: 00000018
 DS:     002B   ES:     002B   FS:     0053   GS:     002B


CS:EIP  0F B6 00 83 E0 01 0F 95 C0 22 C0 74 11 8B C6 83

Note: one could either ignore the error messages that result from the missing MPIR structure definition or adjust gmp-impl.h to always use the little-endian definition (which I've locally did with changing the #if ... sequence to #if .. #elif ... #else).

@LADSoft
Copy link
Owner

LADSoft commented Oct 14, 2019

now it compiles a bunch of stuff; when it gets to DOCs it does this:

   restore=: && backupdir=".am$$$" && am__cwd=`pwd` && CDPATH="${ZSH_VERSION+.}:" && cd ../../doc && rm -rf $backupdir && mkdir $backupdir && if (/bin/sh /c/dev/mpir-3.0.0/missing makeinfo --version) >/dev/null 2>&1; then for f in ../../doc/mpir.info ../../doc/mpir.info-[0-9] ../../doc/mpir.info-[0-9][0-9] .i[0-9] .i[0-9][0-9]; do if test -f $f; then mv $f $backupdir; restore=mv; else :; fi; done; else :; fi && cd "$am__cwd" ; if /bin/sh /c/dev/mpir-3.0.0/missing makeinfo -I ../../doc -o ../../doc/mpir.info ../../doc/mpir.texi; then rc=0; CDPATH="${ZSH_VERSION+.}:" && cd ../../doc; else rc=$?; CDPATH="${ZSH_VERSION+.}:" && cd ../../doc && $restore $backupdir/* `echo "./../../doc/mpir.info" | sed 's|[^/]*$||' `; fi; rm -rf $backupdir; exit $rc

/c/dev/mpir-3.0.0/missing: line 81: makeinfo: command not found
WARNING: 'makeinfo' is missing on your system.
You should only need it if you modified a '.texi' file, or
any other file indirectly affecting the aspect of the manual.
You might want to install the Texinfo package:
https://www.gnu.org/software/texinfo/
The spurious makeinfo call might also be the consequence of
using a buggy 'make' (AIX, DU, IRIX), in which case you might
want to install GNU make:
https://www.gnu.org/software/make/

@GitMensch
Copy link
Contributor Author

As noted this should not happen, and may be an omake issue. In any case you should be able to "skip" this directory by either running omake in the subdirectories excluding "doc" or using omake -k.

LADSoft added a commit that referenced this issue Oct 16, 2019
@LADSoft LADSoft changed the title assembler label problem assembler label problem and work on MPIR Apr 26, 2020
@LADSoft
Copy link
Owner

LADSoft commented May 9, 2020

the only reason i was holding this open is because of my comment that the docs wouldn't build for mpir, and the resulting comment from @GitMensch that maybe there was a bug in omake.

I was able to verify tonite that right now the mpir docs directory does compile. I think at some point I figured out how to install makeinfo? The rest of the project compiles right up to the point where it tries to link the DLL. Since this is likely due to the usage of yasm I'm going to close this.

@LADSoft LADSoft closed this as completed May 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants