[Bug rtl-optimization/44364] New: Wrong code with e500 double floating point

[Bug rtl-optimization/44364] New: Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

The testcase (which is stripped down perl code) attached compiled with:
gcc -fPIC -fno-strict-aliasing -pipe \
         -O2 \
         -g -o tc-lossings-floats tc-lossings-floats.c -Wall -mno-isel

results in after executing:
|RESET: 252.000000 | 0.000000
|RESET: 504.000000 | 252.000000
|RESET: 756.000000 | 504.000000
|RESET: 1008.000000 | 756.000000
|RESET: 1260.000000 | 1008.000000
|RESET: 1512.000000 | 1260.000000
|RESET: 1764.000000 | 1512.000000
|RESET: 2016.000000 | 1764.000000
|RESET: 2268.000000 | 2016.000000
 => 2268.000000
|RESET: 2520.000000 | 0.000000
|RESET: 2772.000000 | 2520.000000

With -O1 instead -O2:
|RESET: 252.000000 | 0.000000
|RESET: 504.000000 | 252.000000
|RESET: 756.000000 | 504.000000
|RESET: 1008.000000 | 756.000000
|RESET: 1260.000000 | 1008.000000
|RESET: 1512.000000 | 1260.000000
|RESET: 1764.000000 | 1512.000000
|RESET: 2016.000000 | 1764.000000
|RESET: 2268.000000 | 2016.000000
 => 2268.000000
|RESET: 2520.000000 | 2268.000000
|RESET: 2772.000000 | 2520.000000

The "=>" line sets the the second value in the "|RESET" line. With -O1 is
remains where it is, with -O2 it gets overwritten. The original perl code gets
here a totally random values. 

Now. It is getting better. The source compiled with -S and the resulting
assembly file assembled with gcc-4.3 does not show this problem. After diffing
of the two resulting binaries I saw  a difference in __floatdidf() which is
called from Kino_OutStream_tell(). The variant which is attached by the 4.3
compiler does not use floating point instead it uses integer code which calls
other functions (__floatsidf, __muldf3, __floatunsidf, __adddf3) which use also
interger code. The 4.3 compiler was not compiled with --enable-e500_double. 

So after looking at the code I saw now the following:
10000c24 <__floatdidf>:
10000c6c:       11 23 1a 2c     evmergehi r9,r3,r3

This function is touching the complete 64bit r9 register
The code which calls it:

10000a40:       91 21 00 20     stw     r9,32(r1)
10000a44:       4e 80 04 21     bctrl   # tell function which in turn calls
floatdidf

10000a7c:       81 21 00 20     lwz     r9,32(r1)
10000a80:       38 60 00 00     li      r3,0    
10000a84:       7e 33 8b 78     mr      r19,r17 
10000a88:       12 49 92 e1     efdsub  r18,r9,r18
10000a8c:       10 80 92 fa     efdctsiz r4,r18
10000a90:       12 49 4a 17     evmr    r18,r9  

So we just save the lower 32bit of r9 before calling the function and the upper
32bit are overwritten by efdsub.


-- 
           Summary: Wrong code with e500 double floating point
           Product: gcc
           Version: 4.4.4
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: gcc at breakpoint dot cc
 GCC build triplet: powerpc-linux-gnuspe
  GCC host triplet: powerpc-linux-gnuspe
GCC target triplet: powerpc-linux-gnuspe


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug rtl-optimization/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #1 from gcc at breakpoint dot cc  2010-06-01 13:42 -------
Created an attachment (id=20793)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20793&action=view)
test case


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug rtl-optimization/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #2 from gcc at breakpoint dot cc  2010-06-01 13:42 -------
Created an attachment (id=20794)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20794&action=view)
objdump of the compiled testcase


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #3 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 19:26 -------
(In reply to comment #0)
> So after looking at the code I saw now the following:
> 10000c24 <__floatdidf>:
> 10000c6c:       11 23 1a 2c     evmergehi r9,r3,r3
> 
> This function is touching the complete 64bit r9 register
> The code which calls it:
> 
> 10000a40:       91 21 00 20     stw     r9,32(r1)
> 10000a44:       4e 80 04 21     bctrl   # tell function which in turn calls
> floatdidf
> 
> 10000a7c:       81 21 00 20     lwz     r9,32(r1)
> 10000a80:       38 60 00 00     li      r3,0    
> 10000a84:       7e 33 8b 78     mr      r19,r17 
> 10000a88:       12 49 92 e1     efdsub  r18,r9,r18
> 10000a8c:       10 80 92 fa     efdctsiz r4,r18
> 10000a90:       12 49 4a 17     evmr    r18,r9  
> 
> So we just save the lower 32bit of r9 before calling the function and the upper
> 32bit are overwritten by efdsub.

Ok, here's the e500 ABI user guide:
  http://www.freescale.com/files/32bit/doc/ref_manual/E500ABIUG.pdf

According to that guide, we have the following usage of GPRs:
  Dedicated: r1 r2 r13
  Nonvolatile: r14-r31
  Volatile: r0 r3-r12

Then:
  Before a function changes the value in the upper word of any nonvolatile
general register, rn, it shall save the 64-bit value in rn in the 64-bit
general register save area 8*(32-n) bytes below the CR save area (plus any
required padding).  The 64-bit general save area shall have 8-byte alignment.

And:
  Before a function changes the value in the lower word of any nonvolatile
general register, rn, that has not already been saved in the 64-bit general
register save area, it shall save the value in the lower word of rn in the word
in the 32-bit general register save area 4*(32-n) bytes before the back chain
word of the previous frame.

So clearly the caller's assembly is wrong; it should be saving all 64-bits of
r9 (volatile gpr) first.

Cheers,
Kyle Moffett


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #4 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 20:09 -------
Ok, I have a trivial 19-line testcase that triggers the bug on my native Debian
GCC 4.4.4-2+powerpcspe1 (with PR44169 fix) with -O0 and -O3.  The compiler was
built with: --with-cpu=8548 --enable-e500_double --with-long-double-128
  In order to make it work on -O3 I had to split the "double z", "void
problem()" and "int main()" into 3 separate files to prevent GCC from inlining
everything.

Compiled with:
  gcc -Wall -Wextra -Werror -ggdb3 -O0 -o testall testall.c
Output (the two "main: a" lines should read the same):
  main: a = 78.463000
  problem: a = 39.231500
  main: a = 39.231476

Or with:
  gcc -Wall -Wextra -Werror -ggdb3 -O3 -o test.o -c test.c
  gcc -Wall -Wextra -Werror -ggdb3 -O3 -o test2.o -c test2.c
  gcc -Wall -Wextra -Werror -ggdb3 -O3 -o test3.o -c test3.c
  gcc -Wall -Wextra -Werror -ggdb3 -O3 -o test test.o test2.o test3.o
Output:  (again, "main" lines should be identical)
  main: a = 78.463000
  problem: a = 39.231500
  main: a = 78.462952

As you might imagine, a bug this bad triggers testsuite failures in MPFR, GMP,
PostgreSQL, lapack, and basically everything else with a comprehensive
floating-point test-suite.

Cheers,
Kyle Moffett


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #5 from gcc at breakpoint dot cc  2010-06-03 20:17 -------
>So clearly the caller's assembly is wrong; it should be saving all 64-bits of
>r9 (volatile gpr) first.

Yes, that it what I've been pointing out. There is an optimization in the stack
code which uses 32bit stores/loads if no double types are used.
I looks to me like the functions branch and register saves are done by the RTL
and it does not know about the 64bit GPRs here. This is only a guess, I'm not a
gcc guy.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #6 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 20:21 -------
Created an attachment (id=20823)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20823&action=view)
Combined trivial testcase (For -O0)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #7 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 20:22 -------
Created an attachment (id=20824)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20824&action=view)
Multipart trivial testcase (For -O3)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #8 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 20:22 -------
Created an attachment (id=20825)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20825&action=view)
Multipart trivial testcase (For -O3) part 2


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #9 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 20:22 -------
Created an attachment (id=20826)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20826&action=view)
Multipart trivial testcase (For -O3) part 3


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #11 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 20:26 -------
Created an attachment (id=20828)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20828&action=view)
Multipart trivial testcase objdump result (Built with -O3)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #10 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 20:26 -------
Created an attachment (id=20827)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20827&action=view)
Multipart trivial testcase objdump result (Built with -O0)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #12 from Kyle dot D dot Moffett at boeing dot com  2010-06-03 23:18 -------
(From update of attachment 20823)
Scratch my test cases... "register asm(...)" doesn't work the way I thought it
did... Sebastian's test case is the only one that I've found that works.


-- 

Kyle dot D dot Moffett at boeing dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20823|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #14 from Kyle dot D dot Moffett at boeing dot com  2010-06-04 01:37 -------
Created an attachment (id=20830)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20830&action=view)
Updated tc-lossings-floats.objdump


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #13 from Kyle dot D dot Moffett at boeing dot com  2010-06-04 01:37 -------
Created an attachment (id=20829)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20829&action=view)
Updated tc-lossings-floats.c

I sat down with GCC and vim for a couple hours narrowing down Sebastian's
testcase.  This file of ~100 lines exhibits the exact same problem as the
original testcase.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #15 from Kyle dot D dot Moffett at boeing dot com  2010-06-04 02:17 -------
Created an attachment (id=20831)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20831&action=view)
Even tinier tc-lossings-floats.c

Spent a bit more time on the testcase and made it even smaller.  The bug goes
away if I make any of the following changes:

  (1) Make "counter" a nonstatic local variable or "iter" a static global
  (2) Call dotell() directly instead of through a function pointer
  (3) Eliminate the references to frq_out->buf_start or prx_out->buf_start,
even though they are always zero.


-- 

Kyle dot D dot Moffett at boeing dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20829|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #16 from Kyle dot D dot Moffett at boeing dot com  2010-06-04 02:17 -------
Created an attachment (id=20832)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20832&action=view)
Even tinier tc-lossings-floats.objdump


-- 

Kyle dot D dot Moffett at boeing dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20830|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #17 from Kyle dot D dot Moffett at boeing dot com  2010-06-04 17:24 -------
Created an attachment (id=20840)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20840&action=view)
Minimal test with -O1

I've managed to shrink this down to a 44-line testcase that fails with the
options listed below.  The code is the same but the failure mode is different; 
Specifically, I get this output:
|RESET: 252.000000 | 4294967296.000000
|RESET: 504.000000 | 4294967296.000000
|RESET: 756.000000 | 4294967296.000000
|RESET: 1008.000000 | 4294967296.000000
|RESET: 1260.000000 | 4294967296.000000
|RESET: 1512.000000 | 4294967296.000000
|RESET: 1764.000000 | 4294967296.000000
|RESET: 2016.000000 | 4294967296.000000
|RESET: 2268.000000 | 4294967296.000000
 => 2268.000000
|RESET: 2520.000000 | 4294967296.000000
|RESET: 2772.000000 | 4294967296.000000

>From the code, the second floating point value on each line should be equal to
the first float from the previous line.  If I change the first "(unsigned long
long)" cast to a "(long long)" cast, the bug goes away and I see the following
(correct) output:
|RESET: 252.000000 | 0.000000
|RESET: 504.000000 | 252.000000
|RESET: 756.000000 | 504.000000
|RESET: 1008.000000 | 756.000000
|RESET: 1260.000000 | 1008.000000
|RESET: 1512.000000 | 1260.000000
|RESET: 1764.000000 | 1512.000000
|RESET: 2016.000000 | 1764.000000
|RESET: 2268.000000 | 2016.000000
 => 2268.000000
|RESET: 2520.000000 | 2268.000000
|RESET: 2772.000000 | 2520.000000

The full list of compile options:
        -pipe \
        -g \
        -Wall \
        -fmove-loop-invariants \
        -fcaller-saves \
        --param max-aliased-vops=100 \
        --param avg-aliased-vops=1 \
        --param max-fields-for-field-sensitive=0 \
        --param loop-invariant-max-bbs-in-loop=1000 \
        -O \
        -fno-web \
        -fno-rename-registers \
        -fno-unroll-loops \
        -fno-peel-loops \
        -fno-split-ivs-in-unroller \
        -fno-peephole \
        -fno-dce \
        -fno-dse \
        -fno-unit-at-a-time \
        -fno-omit-frame-pointer \
        -fno-auto-inc-dec \
        -fno-cprop-registers \
        -fno-dce \
        -fno-defer-pop \
        -fno-delayed-branch \
        -fno-dse \
        -fno-guess-branch-probability \
        -fno-if-conversion2 \
        -fno-if-conversion \
        -fno-inline-small-functions \
        -fno-inline-functions-called-once \
        -fno-early-inlining \
        -fno-ipa-pure-const \
        -fno-ipa-reference \
        -fno-merge-constants \
        -fno-split-wide-types \
        -fno-tree-builtin-call-dce \
        -fno-tree-ccp \
        -fno-tree-ch \
        -fno-tree-copyrename \
        -fno-tree-dce \
        -fno-tree-dominator-opts \
        -fno-tree-dse \
        -fno-tree-fre \
        -fno-tree-sra \
        -fno-tree-ter \
        -fno-merge-constants \
        -fno-branch-count-reg \
        -fno-function-cse \
        -fno-thread-jumps \
        -fno-gcse \
        -fno-rerun-cse-after-loop \
        -fno-cse-skip-blocks \
        -fno-cse-follow-jumps


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #18 from Kyle dot D dot Moffett at boeing dot com  2010-06-04 17:24 -------
Created an attachment (id=20841)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20841&action=view)
Minimal test objdump with -O1


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gmail dot com]

------- Comment #19 from amodra at gmail dot com  2010-06-06 14:11 -------
Confirmed.

Regarding O1test.c:
Wierd set of gcc options, particularly -fno-dce and -fcaller-saves.  I can't
see any sane reason why you would use those options on powerpc, unless you were
deliberately stress testing gcc..  It's quite possible that your O1test.c is
tickling an entirely different bug than in the original testcase.  Trimming the
options a bit, I can reproduce the O1test.c problem with only
-fcaller-saves -O -fno-omit-frame-pointer -fno-dce -fno-split-wide-types


-- 

amodra at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-06-06 14:11:46
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gmail dot com]

------- Comment #20 from amodra at gmail dot com  2010-06-06 14:52 -------
My guess is that tc-lossings-floats.c hits an ira related problem, but I'm not
particularly familiar with that area of the compiler so won't look further
myself.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #21 from Kyle dot D dot Moffett at boeing dot com  2010-06-06 15:37 -------
I was trying to strip down the testcase and try to see which optimization flags
caused it.  I started from -O2 and tried to see which -O2 flags (in addition to
O1) were needed to cause the problem.  From there I simplified the testcase as
small as I could get it and turned off as many other optimizations as possible.
 As far as I can tell, it seems to be a combination of -fcaller-saves and the
minimal CSE that -O1 always turns on (regardless of other -fno-* options).

Part of the reason for the extra -fno-* compiler options was to prevent the
compiler from moving registers around so much as I fiddled with the testcase,
since the bug is only triggered when it allocates one of the 2 doubles in a
register that is reused by one of the libgcc functions it calls.

Other than that, I'm just grasping at straws...

If somebody can debug this and fix both testcases before FreeScale gets back to
us with a fix I'll ship them a case or two of their favorite beer (or whatever
other beverage they'd prefer).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gmail dot com]

------- Comment #22 from amodra at gmail dot com  2010-06-07 04:41 -------
Adding the following to config/rs6000/e500.h will likely fix the bug. 
Testing..

#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
  (TARGET_E500_DOUBLE && ((MODE) == DFmode || (MODE) == TFmode          \
                          || (MODE) == DCmode || (MODE) == TCmode)      \
   ? (MODE)                                                             \
   : choose_hard_reg_mode ((REGNO), (NREGS), false))


-- 

amodra at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |amodra at gmail dot com
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2010-06-06 14:11:46         |2010-06-07 04:41:52
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #23 from Kyle dot D dot Moffett at boeing dot com  2010-06-07 05:36 -------
(In reply to comment #22)
> Adding the following to config/rs6000/e500.h will likely fix the bug. 
> Testing..

Oh, very nice!  Thanks for the speedy assistance on this!  I've got my own test
compiler building now; I'll let you know what the result is as soon as its
done!

Cheers,
Kyle Moffett


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #24 from Kyle dot D dot Moffett at boeing dot com  2010-06-07 05:44 -------
Hmm, unfortunately in my preliminary testing this does not seem to fix either
testcase.

Cheers,
Kyle Moffett


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gmail dot com]

------- Comment #25 from amodra at gmail dot com  2010-06-07 09:53 -------
Yes it seems the patch is not sufficient on 4.4.  On mainline the code looks
good by inspection.  (I don't have e500 hardware to run tests on.)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gmail dot com]

------- Comment #26 from amodra at gmail dot com  2010-06-07 10:29 -------
Doh!  No, it's still broken on mainline too.  I wasn't testing what I thought I
was...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #27 from Kyle dot D dot Moffett at boeing dot com  2010-06-07 12:49 -------
(In reply to comment #25)
> Yes it seems the patch is not sufficient on 4.4.  On mainline the code looks
> good by inspection.  (I don't have e500 hardware to run tests on.)

If you'd like login access to one of our protoboards, just post an SSH public
key on bugzilla and we'll set up port-forwarded access to one of our Debian
buildd servers.  We're thankful for all the assistance you've been able to
provide.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gmail dot com]

------- Comment #28 from amodra at gmail dot com  2010-06-07 17:05 -------
Please bootstrap and test this addition to e500.h

/* When setting up caller-save slots (MODE == VOIDmode) ensure we
   allocate space for DFmode.  Save gprs in the correct mode too.  */
#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
  (TARGET_E500_DOUBLE && ((MODE) == VOIDmode || (MODE) == DFmode)       \
   ? DFmode                                                             \
   : choose_hard_reg_mode ((REGNO), (NREGS), false))


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #29 from Kyle dot D dot Moffett at boeing dot com  2010-06-07 18:28 -------
Awesome!!! Both of our testcases that were failing pass with this patch
applied!

I'm not going to call it a 100% victory yet, I want to rebuild our native
compilers and build-and-run the PostgreSQL, GMP and MPFR testsuites first.

Here's hoping though! *crosses fingers*


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #30 from Kyle dot D dot Moffett at boeing dot com  2010-06-07 18:56 -------
Ok, the cross-compiler built with this patch fails to build a native GCC for
the target with the following ICE:

../../../src/libgcc/../libdecnumber/decLibrary.c: In function 'isinfd128':
../../../src/libgcc/../libdecnumber/decLibrary.c:71: error: unrecognizable
insn: (insn 6 5 7 2 ../../../src/libgcc/../libdecnumber/decLibrary.c:68 (set
(subreg:TI (reg:TD 122 [ arg ]) 0) (reg:TI 123)) -1 (nil))
../../../src/libgcc/../libdecnumber/decLibrary.c:71: internal compiler error:
in extract_insn, at recog.c:2001


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #31 from Kyle dot D dot Moffett at boeing dot com  2010-06-08 04:24 -------
Alan,

Now that I've corrected all of my compiler issues, your first patch (the one
quoted below) works like a charm.  I still need to do the extensive
archive-rebuild and testsuite run to verify that it completely works, but that
will take a couple days.

Looking good!

Cheers,
Kyle Moffett

(In reply to comment #28)
> /* When setting up caller-save slots (MODE == VOIDmode) ensure we
>    allocate space for DFmode.  Save gprs in the correct mode too.  */
> #define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
>   (TARGET_E500_DOUBLE && ((MODE) == VOIDmode || (MODE) == DFmode)       \
>    ? DFmode                                                             \
>    : choose_hard_reg_mode ((REGNO), (NREGS), false))


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #32 from Kyle dot D dot Moffett at boeing dot com  2010-06-08 05:07 -------
The first working patch (VOIDmode||DFmode) just successfully passed full GMP
and MPFR testsuites on the e500 boards.  PPL and EGLIBC are currently
rebuilding.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #33 from Kyle dot D dot Moffett at boeing dot com  2010-06-08 06:48 -------
EGLIBC and PPL are still building; I'm heading to sleep and I'll check on them
later this morning.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #34 from gcc at breakpoint dot cc  2010-06-08 11:23 -------
(In reply to comment #28)
> #define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
>   (TARGET_E500_DOUBLE && ((MODE) == VOIDmode || (MODE) == DFmode)       \
>    ? DFmode                                                             \
>    : choose_hard_reg_mode ((REGNO), (NREGS), false))
> 
I applied it on 4.4 branch. The big Perl thing is passes its testsuite with
this.
Also I don't see evldd X, stw X, ld X constructs, there is just one evldd. It 
looks good so far.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #35 from Kyle dot D dot Moffett at boeing dot com  2010-06-08 15:23 -------
Hrm, well PPL still seems to be failing the "interval1" test, but I'm not sure
that one's related as the part that fails is "test01<float>".  More info to
come shortly.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #36 from Kyle dot D dot Moffett at boeing dot com  2010-06-08 20:34 -------
Ok, I'm pretty sure this is unrelated to this bug, but I still get one
test-failure with PPL 0.10.2.  The "interval1" test fails due to the
"test01<float>" subtest, apparently due to very slightly excessive "float"
rounding errors (after 100 rounds of the equivalent of "x = (x + (2/x)) / 2" it
approximates ~0.02% past the maximum acceptable error bounds).

The "double" testcase passes just fine (although with the same error bounds and
extra precision that seems perfectly reasonable).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #37 from gcc at breakpoint dot cc  2010-06-09 07:50 -------
Created an attachment (id=20873)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20873&action=view)
this fails to compile in -O2 with the fix


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #38 from gcc at breakpoint dot cc  2010-06-09 07:54 -------
(In reply to comment #28)
> Please bootstrap and test this addition to e500.h
> 
> /* When setting up caller-save slots (MODE == VOIDmode) ensure we
>    allocate space for DFmode.  Save gprs in the correct mode too.  */
> #define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
>   (TARGET_E500_DOUBLE && ((MODE) == VOIDmode || (MODE) == DFmode)       \
>    ? DFmode                                                             \
>    : choose_hard_reg_mode ((REGNO), (NREGS), false))
> 

Okay. Now I found something: 

inst/bin/powerpc-linux-gnuspe-gcc extract_chmLib.i -o extract_chmLib.S -S -O2
extract_chmLib.c: In function '_extract_callback':
extract_chmLib.c:29: internal compiler error: in change_address_1, at
emit-rtl.c:1954
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

extract_chmLib.i is attached. Adding -mfloat-gprs=single which avoids using
64bit gprs for double makes this go away.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[harry dot he at freescale dot com]

------- Comment #39 from harry dot he at freescale dot com  2010-06-09 08:59 -------
Hi,  Kyle Moffett,
    in testall.c, r9 is used by a register variable, however, in E500ABI guide,
r9 should be used for parameter passing, this test case seems not reasonable.

Harry He


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[harry dot he at freescale dot com]

------- Comment #40 from harry dot he at freescale dot com  2010-06-09 09:23 -------
with my toolchain (From CodeSourcery, 4.4-78), o1test gives correct behavior
with built-in flags(-te500v2), but wrong behaviors with "-fcaller-saves -O
-fno-omit-frame-pointer -fno-dce -fno-split-wide-types". Results are same even
after I rebuilt the toolchain with the patch to e500.h.

is there any tricks here? 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gmail dot com]

------- Comment #41 from amodra at gmail dot com  2010-06-09 13:26 -------
Created an attachment (id=20877)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20877&action=view)
e500.h and caller-save.c patch

The ICE in #38 is due to a bug in caller-save.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #42 from gcc at breakpoint dot cc  2010-06-09 13:52 -------
(In reply to comment #41)
> The ICE in #38 is due to a bug in caller-save.c

Thank you for the very quick fix.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[mark dot workman at acm dot org]

------- Comment #43 from mark dot workman at acm dot org  2010-06-09 14:07 -------
(In reply to comment #39)
> Hi,  Kyle Moffett,
>     in testall.c, r9 is used by a register variable, however, in E500ABI guide,
> r9 should be used for parameter passing, this test case seems not reasonable.
> 
> Harry He

Please note that this testcase was removed by Kyle (see comment #12) and that
neither the original testcase (tc-lossings-float.c) nor the trimmed testcase
(tc-lossings-float-3.c) make such explicit use of particular registers.  Thus,
it does appear that it is the compiler that is making the register assignments
in question.

Cheers,
  Mark

> 


-- 

mark dot workman at acm dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark dot workman at acm dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[harry dot he at freescale dot com]

------- Comment #44 from harry dot he at freescale dot com  2010-06-10 09:03 -------
Thanks for your reminding, Mark.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[mark dot workman at acm dot org]

------- Comment #45 from mark dot workman at acm dot org  2010-06-10 17:43 -------
(In reply to comment #40)
> with my toolchain (From CodeSourcery, 4.4-78), o1test gives correct behavior
> with built-in flags(-te500v2), but wrong behaviors with "-fcaller-saves -O
> -fno-omit-frame-pointer -fno-dce -fno-split-wide-types". Results are same even
> after I rebuilt the toolchain with the patch to e500.h.
> 
> is there any tricks here? 
> 

>From what I can tell of CodeSourcery 4.4-78, it contains a heavily patched fork
of gcc 4.4.1.  This bug is reported against gcc 4.4.4.  Harry, are you saying
you believe this is a regression of gcc from 4.4.1 to 4.4.4?  Or does
CodeSourcery have a (perhaps different) patch for this issue that should be
submitted here?

Regards,
  Mark


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[harry dot he at freescale dot com]

------- Comment #46 from harry dot he at freescale dot com  2010-06-11 01:09 -------
Mark, I have reported the issue to CodeSourcery and waiting them to confirm, I
guess our toolchain may need different patches for it, or there is something
wrong in my validation.


-- 

harry dot he at freescale dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |harry dot he at freescale
                   |                            |dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[Kyle dot D dot Moffett at boeing dot com]

------- Comment #47 from Kyle dot D dot Moffett at boeing dot com  2010-06-21 15:55 -------
(In reply to comment #41)
> Created an attachment (id=20877)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20877&action=view) [edit]
> e500.h and caller-save.c patch
> 
> The ICE in #38 is due to a bug in caller-save.c

We've basically rebuilt everything in the PowerPCSPE Debian port now, and as
far as I can tell all of the test-cases affected by this bug are fixed by your
attachment 20877 (pr44364.diff).  Now that we've gotten further, Sebastian has
identified a new floating point bug (PR44606), but I believe we can call this
one closed.

Again, many thanks for all your help!

Cheers,
Kyle Moffett


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[amodra at gcc dot gnu dot org]

------- Comment #48 from amodra at gcc dot gnu dot org  2010-06-22 10:44 -------
Subject: Bug 44364

Author: amodra
Date: Tue Jun 22 10:44:11 2010
New Revision: 161163

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161163
Log:
        PR target/44364
        * config/rs6000/e500.h (HARD_REGNO_CALLER_SAVE_MODE): Define.
        * caller-save.c (insert_restore, insert_save): Use non-validate
        form of adjust_address.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/caller-save.c
    trunk/gcc/config/rs6000/e500.h


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364

[Bug target/44364] Wrong code with e500 double floating point

[gcc at breakpoint dot cc]

------- Comment #49 from gcc at breakpoint dot cc  2010-06-28 11:18 -------
>Modified:
>    trunk/gcc/ChangeLog
>    trunk/gcc/caller-save.c
>    trunk/gcc/config/rs6000/e500.h

Is it possible to get this into the 4.4 and 4.5 branch as well?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44364