AW: 68000 issue in longlong.h

[se...@t-online.de]

If you could show the generated code (after gcc's register allocation)
*and* point out precisely where things go wrong, that would be helpful.

Regards,
/Niels


The generated code is on the compiler explorer link I provided:
http://franke.ms/cex/z/oG53bK 

There you see the original code on the left and the generated assembly in the 
middle.
You can add the & to
"=d" (__umul_tmp1)
in the left window and you see immediately the change in the comiled output.


I am not the expert here and I do not want to complain or bother you at all. It 
was just my observation of symptoms I wanted to report.
The compiler maintainer says "gcc is ok, the problem is in longlong.h"
You say "longlong.h looks ok, we wouldn't hide compiler bugs."

For me the workaround (add &) works and all tests pass with it, so can live 
with the workaround. But somewhere there must be a bug/glitch. If you could 
prove gcc to be wrong that would also be a really great result!


Thanks for all your effort

Best regards Alexander

___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

AW: 68000 issue in longlong.h

[se...@t-online.de]

>  With his change the tests pass so I think the change is reasonable.

>I disagree.  If I am right that the inline asm is correct, changing it
>to avoid triggering a compiler bug is not a reasonable change.  But
>perhaps I am wrong and the inline asm is buggy, but then somebody needs
>to explain in what way it is buggy.
 
>Torbjörn


Of course I do not want to hide a compiler error here and disturb other 
platforms. If this turns out to be a compiler error that would be a nice 
finding as well!


I looked in https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

>6.47.2.6 Clobbers and Scratch Registers
>
>Rather than allocating fixed registers via clobbers to provide scratch 
>registers for an asm statement, an alternative is to define a variable and 
>make it an early-clobber output.

I think that is done in the inline assembler code with the variables 
USItype __umul_tmp1, __umul_tmp2;

To make them an "early clobber output" the & sign has to be written before the 
d as it has been done for __umul_tmp2.

>6.47.2.3 Output Operands
>Use the ‘&’ constraint modifier (see Modifiers) on all output operands that 
>must not overlap an input. Otherwise, GCC may allocate the output operand in 
>the same register as an unrelated >input operand, on the assumption that the 
>assembler code consumes its inputs before producing outputs. This assumption 
>may be false if the assembler code actually consists of more than one 
>instruction.

If you look at the nice compiler explorer at 
https://franke.ms/cex/z/oG53bK
you see the code generated for -O1 and -O0. If you add the % sign in the left 
window you will imediatelly see the changed code in the middle window.

Can you explain why __umul_tmp1 should not be an "early-clobber output"?

Btw,
Could you tell me what the "%2" at the input parameters line means? I did not 
find that in the Extended-Asm.html document.

: "%2" ((USItype)(a)), "d" ((USItype)(b)));

Kind regards Alexander
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

AW: 68000 issue in longlong.h

[se...@t-online.de]

   
>The current code looks correct to me.  Please explain why you think it
>is not correct.
>
>
>-- 
>Torbjörn

Hello,
thank you for looking into my report.

I am not an assembler expert. I compiled the lib with and without -O2 and found 
without -O2 the tests worked. So I started compiling the c-files with and 
without -O2 to find out the problematic file. I eventually found exactly one 
file. If all others in the lib were compiled with -O2 and only 
mpn/sqr_basecase.c without optimization, then the tests passed. So I suspceted 
a bug in Amiga gcc6 and asked the maintainer of amiga gcc6.

He tried and answered on https://github.com/bebbo/gcc/issues/145 
This is his answer:

>I disabled the asm mult16x16 to hunt the bug and with the generic version it 
>run well. So the problem had to be the asm.
>Here I compared the generated asm: http://franke.ms/cex/z/oG53bK and you see 
>that only one register is used instead of two, since the modification is not 
>recognized.
>
>/* here --> */ "=d" (__umul_tmp1),
>
>to
>
>/* here --> */ "=&d" (__umul_tmp1),
>
>does the magic.


With his change the tests pass so I think the change is reasonable.


Kind regards Alexander






___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

68000 issue in longlong.h

[se...@t-online.de]

Hello,
I am compiling and testing libgmp for AmigaOS. Yes, I am crazy.
 
I use
Thanks to bebbo's gcc https://github.com/bebbo/amiga-gcc 
 I was quite successful. I had some 
issues in the tests hovever.
For instance t-conv, t-sqrt and t-sqrt_ui failed. The bugs disappeared when 
I compiled without optimization and appeared when I used -O2, so I 
suspected a compiler bug and reported that to the Amiga gcc6 maintainer. He 
looked after the problem and found an issue inside libgmp's code.
 
The bug is inside gmp's header longlong.h:1220, where asm for 68000 is 
defined:
 
wrong code:
...
" | End inlined umul_ppmm" \
: "=&d" (xh), "=&d" (xl), \
"=d" (__umul_tmp1), "=&d" (__umul_tmp2) \
: "%2" ((USItype)(a)), "d" ((USItype)(b))); \
 
correction:
...
" | End inlined umul_ppmm" \
: "=&d" (xh), "=&d" (xl), \
"=&d" (__umul_tmp1), "=&d" (__umul_tmp2) \<---  an & must be added 
after the =
: "%2" ((USItype)(a)), "d" ((USItype)(b))); \
 
With this correction these tests pass on 68000.
 
Best regards

___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs