Bug 837630 - internal compiler error: in reload_cse_simplify_operands, at postreload.c:403
internal compiler error: in reload_cse_simplify_operands, at postreload.c:403
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: gcc (Show other bugs)
19
ppc Linux
high Severity high
: ---
: ---
Assigned To: Jakub Jelinek
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-04 09:08 EDT by Karsten Hopp
Modified: 2013-07-29 15:01 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-07-29 14:27:58 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
preprocessed source (1.42 MB, text/plain)
2012-07-05 08:24 EDT, Karsten Hopp
no flags Details

  None (edit)
Description Karsten Hopp 2012-07-04 09:08:08 EDT
Description of problem:
/builddir/build/BUILD/gcc-4.7.1-20120629/obj-ppc64-redhat-linux/./prev-gcc/xgcc -B/builddir/build/BUILD/gcc-4.7.1-20120629/obj-ppc64-redhat-linux/./prev-gcc/ -B/usr/ppc64-redhat-linux/bin/ -B/usr/ppc64-redhat-linux/bin/ -B/usr/ppc64-redhat-linux/lib/ -isystem /usr/ppc64-redhat-linux/include -isystem /usr/ppc64-redhat-linux/sys-include    -c   -O2 -g -Wall -fexceptions -fstack-protector --param=ssp-buffer-size=4 -gtoggle -fprofile-generate -DIN_GCC   -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Wold-style-definition -Wc++-compat   -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include -I../../gcc/../libcpp/include  -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/dpd -I../libdecnumber    ../../gcc/gimple-low.c -o gimple-low.o
../../gcc/combine.c: In function 'subst':
../../gcc/combine.c:5318:1: error: insn does not satisfy its constraints:
(insn 4211 1484 1493 140 (set (mem/c:DI (plus:SI (reg/f:SI 19 19 [2736])
                (const_int 32760 [0x7ff8])) [3 __gcov0.subst+816 S8 A64])
        (reg:DI 6 6)) 399 {*movdi_internal32}
     (nil))
../../gcc/combine.c:5318:1: internal compiler error: in reload_cse_simplify_operands, at postreload.c:403

Version-Release number of selected component (if applicable):
gcc-4.7.1-1.fc18

Steps to Reproduce:
1. ppc-koji build --scratch f18 gcc-4.7.1-1.fc18.src.rpm
2.
3.
  
Actual results:


Expected results:


Additional info:
Comment 1 Karsten Hopp 2012-07-05 08:23:17 EDT
this issue exists on PPC only, ppc64 builds fine.
Comment 2 Karsten Hopp 2012-07-05 08:24:56 EDT
Created attachment 596395 [details]
preprocessed source
Comment 3 Alan Modra 2012-07-08 22:33:37 EDT
Some notes:
1) The instruction is actually good.  It can and should generate stw 6,32760(19); stw 7,32764(19).
2) The failing constraint is "o<>", specifically "o" so offsettable_memref_p is involved.  It fails because rs6000_mode_dependent_address rejects any offset greater or equal to 32756.  The reason for that is to support splitting of TImode to 4 SImode pieces, addressable at off+0, off+4, off+8 and off+12.  rs6000_mode_dependent_address uses the worst case because it doesn't have the mode, in this case DImode.
3) The instruction predicate correctly allows an offset of 32760.
4) rth must have had a bad day when he wrote "val + 12 + 0x8000 >= 0x10000" in rs6000_mode_dependent_address.  That allows a range of -32780 to 32755.  Correct is "val + 0x8000 >= 0x10000 - 12".
5) As far as offsettable_memref_p is concerned, the offset range check in rs6000_mode_dependent_address is completely redundant.  See ycode==PLUS checks in offsettable_address_addr_space_p.

(2) and (3) together is why we have a problem, since the insn constraints don't cover all possibilities allowed by the insn predicate.  So, why don't we simply use "m" in movdi_internal32 in place of "o<>"?  I can't see any later constraint pairs that would match the non-offsettable parts of "m".  Bootstrap and regression test with that change plus the fix in (4) show no regressions, and cure the testcase.
Comment 4 Jakub Jelinek 2012-07-09 04:13:47 EDT
I wonder if we shouldn't use in movdi_internal32 simply a predicate that for MEM checked offsettable_mem_operand (or of course, change mode_dependent_address hook to include MEM mode too).
Comment 5 Alan Modra 2012-07-09 07:43:01 EDT
Incidentally, the "o<>" constraint in movdi_internal32 came in with revision 85419.  http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00022.html
Geoff patched four insns, one of which, movtf_internal, has since had "o<>" replaced with "Y".  Since the patch was regarding long double support on Darwin, I can understand poking at ti and tf patterns, but I wonder why Geoff did anything with a di pattern?

Regarding comment #4, I think I prefer relaxing the constraint rather than tightening the predicate.  Obviously tightening the predicate would work, but as I said in comment #2 the insn is good.

What's wrong with using "m" instead of "o<>"?  Can reload generate a mem that doesn't satisfy the operand predicate, and not detect it?
Comment 6 Alan Modra 2012-07-10 00:28:51 EDT
Re comment #2 point 4.  More correct is
"val + 0x8000 >= 0x10000 - (TARGET_POWERPC64 ? 8 : 12)"
Comment 7 Jakub Jelinek 2012-07-10 04:09:23 EDT
As long as something like
void
foo (void *x, long long y)
{
  asm ("#": "+r" (y)
       : : "fr0", "fr1", "fr2", "fr3", "fr4", "fr5", "fr6", "fr7",
           "fr8", "fr9", "fr10", "fr11", "fr12", "fr13", "fr14", "fr15",
           "fr16", "fr17", "fr18", "fr19", "fr20", "fr21", "fr22", "fr23",
           "fr24", "fr25", "fr26", "fr27", "fr28", "fr29", "fr30", "fr31");
  *(long long *) (x + 32764) = y;
}
works even with "m" instead of "o<>", fine.  Seems this works because
+ 32764 is not rs6000_legitimate_offset_address_p.
Comment 8 Alan Modra 2012-07-10 08:28:08 EDT
Yes, the above compiles fine, both as 32-bit and 64-bit, with and without the asm.

Also very interesting is to look at -m32 -O2 output for the following where we get quite a lot better code for f0 thru f3 using "m" instead of "o<>".

void
f0 (void *x, long long y)
{
  *(long long *) (x + 32760) = y;
}

void
f1 (void *x, long long y)
{
  *(long long *) (x + 32761) = y;
}

void
f2 (void *x, long long y)
{
  *(long long *) (x + 32762) = y;
}

void
f3 (void *x, long long y)
{
  *(long long *) (x + 32763) = y;
}

void
f4 (void *x, long long y)
{
  *(long long *) (x + 32764) = y;
}

void
f5 (void *x, long long y)
{
  *(long long *) (x + 32765) = y;
}

void
f6 (void *x, long long y)
{
  *(long long *) (x + 32766) = y;
}

void
f7 (void *x, long long y)
{
  *(long long *) (x + 32767) = y;
}
Comment 9 Alan Modra 2012-07-16 11:33:58 EDT
Looking at this again today I found some quite odd things in the reload dump.  The failing memory store started out as insn 1485, and gcc was supposed to reload the reg to fr0, which is a dumb thing to do but a correct choice none the less.  Current mainline does just that.  However on gcc-4.7, insn 1485 was deleted!  This happened at reload1.c:1041, based on the reg_renumber and reg_equiv_init.  Hmm, didn't we have a bug related to resizing this vec?  Or perhaps http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01688.html and its followup?  Ah, no, that patch was for find_moveable_pseudos which we don't have on 4.7.  Still, something is very fishy with reg_equiv_init to have it point at the insn where the pseudo dies.
Comment 10 Alan Modra 2012-07-16 23:49:13 EDT
What's happening here is that the pseudo for the reg in insn 1485 doesn't get a hard reg.  Nothing unusual in that, but gcc also knows it has an equiv mem location.  On mainline we come out of the reload loop with the pseudo being assigned a gpr.  On 4.7 is doesn't have a reg at all, ie. lives in mem.  I think this is more or less just luck of the draw, so the same thing could happen on mainline with a slightly different test case.  The problem is that reload uses the mem for the pseudo without any concern for insn constraints.  (Well, not entirely, but gen_reload in this case just uses gen_move_insn.)  This in turn means that any mem that might be used by reload must have a matching load/store insn in the machine description.  rs6000.md doesn't, due to the "o" constraint problem mentioned in comment #3.
Comment 11 Karsten Hopp 2012-07-17 05:06:48 EDT
As I have noe clue what all this is about, I need your advice.  Is this something we have to worry about ? We have a mass rebuild of all our packages coming up, does this issue need to be fixed before that ?
Comment 12 Jakub Jelinek 2012-07-17 05:13:19 EDT
For the time being I've applied the small change on *movdi_internal32 from "=o<>,..." to "=m,..." in the constraint (the change is in gcc-4.7.1-3.fc18).
It passed bootstrap/regtest, hope it works fine temporarily and for the final solution I'll wait for Alan's commits.
Comment 13 IBM Bug Proxy 2012-11-09 12:32:01 EST
Alan's patched have been committed for a while now.  Are we going to live with Jakub's "=m,..." workaround or has Alan's patches been merged into the F18 gcc tree yet?
Comment 14 Fedora End Of Life 2013-04-03 14:09:08 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19
Comment 15 Peter Bergner 2013-07-26 12:34:22 EDT
I'm trying to clean up bugs on the IBM side and came across this one.  Can this bug be closed as fixed, since we're either using Jakub's workaround or Alan's final fix?
Comment 16 Jeff Law 2013-07-29 14:27:58 EDT
Per c#15.
Comment 17 IBM Bug Proxy 2013-07-29 15:01:05 EDT
Closing on the IBM side too.

Note You need to log in before you can comment on or make changes to this bug.