Issue on windows build (mingw64)

[Vlad Gabriel]

Hello Torbjörn,

Unfortunately they are not enough.

For mpn/x86_64/coreisbr/aorsmul_1.asm
Turns out that the missing pop's were indeed causing the SIGSEV. Now, with your modification that routine is crashing anymore, but it gives wrong results, causing various test to fail/assert.
Side note: the fact that the assembler routines do not follow windoze standards (stack usage, prolog, epilogue, etc) make debugging harder, as GDB is no longer able to find the stack frames in order to pinpoint the fault address or know the stack trace. That's why I failed to see that those missing pop's were causing the fault, as the apparent place of the fault was  somewhere random in code.

After a long inspection of the code, understanding the register renaming reasons, I found out the culprit:
IFDOS(`    define(`r8', ``r11'')    ') dnl


Turns out that %r11 was a bad choice, as it was already used in the routine for other purposes.
While not really related, but while searching for a possible fix, I also noticed that %rbx was not used for DOS64, but was saved in prologue. It was an fortunate finding as I used that instead of %r11 :)


For gmp-hg/mpn/x86_64/coreisbr/mul_1.asm
Now, that routine was still giving SIGSEV's. Armed with my previous findings, I searched for missing pop's and it turns out you missed some again :). Fixing those the routine stopped crashing, but was giving bad results. Oh well, start hunting for the offending register rename... It turns out it wasn't any, but a missing move. A register was used (due to renaming) as "n_param" without having "n_param" value, but "up" value.

Now, with all the changes applied, testing was successful. Please find attached coreisbr_aorsmul_mul_dos64.zip containing the patch. I also took the liberty to add the parsed tmp-*.s as a reference.

Personal note and suggestion.
While trying to understand the renaming in each of the above routine coupled with finding the issue, one point I found hard was to keep track of register usage and parameters. Adding to complexity was the fact that sometimes a register was used with it's name (eg %r11), other times with various aliases coming from defines. It would have been more simple (for me) if the same naming was used throughout the source. Therefore I propose to use virtual names inside the code (REG1, REG2...etc) and then conditionally define each for the current calling convention (STD/DOS64). This would make easy to keep track of register usage, which register is clobbered or not, maybe even avoid constructs like "DOS64_ENTRY(4)" or "IFDOS(``push    %rsi        '')"


Best Regards,
Gabriel


________________________________
 From: Torbjorn Granlund <tg at gmplib.org>
To: Vlad Gabriel <clampi at yahoo.com> 
Cc: "gmp-bugs at gmplib.org" <gmp-bugs at gmplib.org> 
Sent: Wednesday, April 18, 2012 12:19 AM
Subject: Re: Issue on windows build (mingw64)
 
Vlad Gabriel <clampi at yahoo.com> writes:

  Regarding issue 3, un-commenting those statements made a lot of test
  fail throughout with SIGSEV. I tried debugging some, but fail to
  understand why they crash, as I had trouble understanding the renaming
  macros purpose. As a note, although I do have the impression that some
  pop's are missing from the epilogue, the crash happens in function
  itself, not when the return is encountered. Please advise if you have
  more thoroughly investigations steps on how to further debug the
  cause.
  
You were right about the missing pops, I fixed those, a more subtle
issue.  With some luck, the code now works.  I have pushed the fixes.

If you could try the new code, first uncommenting the ABI_SUPPORT decls,
I would appreciate it.

-- 
Torbjörn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: coreisbr_aorsmul_mul_dos64.zip
Type: application/zip
Size: 2439 bytes
Desc: not available
URL: <http://gmplib.org/list-archives/gmp-bugs/attachments/20120418/a29a8581/attachment.zip>