gcc -fpic -O1 -c postreload.c Dropping optimization from -O1 to -O0 or dropping -fpic allows this to succeed. Using built-in specs. Target: m68k-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 4.3.1-8' --with-bugurl=file:///usr/share/doc/gcc-4.3/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.3 --program-suffix=-4.3 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --disable-libssp --disable-werror --enable-checking=release --build=m68k-linux-gnu --host=m68k-linux-gnu --target=m68k-linux-gnu Thread model: posix gcc version 4.3.1 (Debian 4.3.1-8) COLLECT_GCC_OPTIONS='-save-temps' '-v' '-fpic' '-O1' '-c' '-m68020' /usr/lib/gcc/m68k-linux-gnu/4.3.1/cc1 -E -quiet -v postreload.c -m68020 -fpic -O1 -fpch-preprocess -o postreload.i ignoring nonexistent directory "/usr/local/include/m68k-linux-gnu" ignoring nonexistent directory "/usr/lib/gcc/m68k-linux-gnu/4.3.1/../../../../m68k-linux-gnu/include" ignoring nonexistent directory "/usr/include/m68k-linux-gnu" #include "..." search starts here: #include <...> search starts here: /usr/local/include /usr/lib/gcc/m68k-linux-gnu/4.3.1/include /usr/lib/gcc/m68k-linux-gnu/4.3.1/include-fixed /usr/include End of search list. COLLECT_GCC_OPTIONS='-save-temps' '-v' '-fpic' '-O1' '-c' '-m68020' /usr/lib/gcc/m68k-linux-gnu/4.3.1/cc1 -fpreprocessed postreload.i -quiet -dumpbase postreload.c -m68020 -auxbase postreload -O1 -version -fpic -o postreload.s GNU C (Debian 4.3.1-8) version 4.3.1 (m68k-linux-gnu) compiled by GNU C version 4.3.1, GMP version 4.2.2, MPFR version 2.3.1. GGC heuristics: --param ggc-min-expand=82 --param ggc-min-heapsize=98603 Compiler executable checksum: 941413abde0b2d7fe947ae106f766200 postreload.c: In function 'do_termsform': postreload.c:745: warning: assignment makes pointer from integer without a cast postreload.c:806: warning: initialization makes pointer from integer without a cast postreload.c:842: warning: passing argument 1 of 'Rf_install' makes pointer from integer without a cast postreload.c:845: warning: passing argument 1 of 'Rf_install' makes pointer from integer without a cast postreload.c:845: warning: assignment makes pointer from integer without a cast postreload.c: At top level: postreload.c:488: warning: 'EncodeVars' used but never defined postreload.c:533: warning: 'ExpandDots' used but never defined postreload.c: In function 'do_termsform': postreload.c:886: error: insn does not satisfy its constraints: (insn 309 2675 2677 36 postreload.c:446 (set (reg:SI 0 %d0) (plus:SI (mem/f:SI (reg:SI 8 %a0) [0 S4 A16]) (reg:SI 0 %d0))) 132 {*addsi3_internal} (nil)) postreload.c:886: internal compiler error: in reload_cse_simplify_operands, at postreload.c:395 Please submit a full bug report, with preprocessed source if appropriate. See <file:///usr/share/doc/gcc-4.3/README.Bugs> for instructions.
Created attachment 16040 [details] Preprocessed file
Created attachment 16041 [details] reduced source from r-base
Caused by: 2007-07-23 Peter Bergner <bergner@vnet.ibm.com> Jakub Jelinek <jakub@redhat.com> PR middle-end/PR28690 Can be reproduced with gcc.c-torture/execute/20060420-1.c when compiled with -O2.
I think there is a missing constrain_operands somewhere, because in (define_insn "*addsi3_internal" [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,d,a") (plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0,0") (match_operand:SI 2 "general_src_operand" "dIKLT,rJK,a,mSrIKLT,mSrIKLs")))] the insn does match the fourth alternative with operands 1 and 2 commuted.
Reverting this change in commutative_operand_precedence fixes the testcase: case RTX_OBJ: /* Complex expressions should be the first, so decrease priority - of objects. */ - return -1; + of objects. Prefer pointer objects over non pointer objects. */ + if ((REG_P (op) && REG_POINTER (op)) + || (MEM_P (op) && MEM_POINTER (op))) + return -1; + return -2; case RTX_COMM_ARITH: /* Prefer operands that are themselves commutative to be first.
But that was the meat of fixing PR28690. :-( The insn should satisfy the constraints of alternative 4.
m68k-linux-gnu is neither primary nor secondary target.
(In reply to comment #6) > But that was the meat of fixing PR28690. :-( > > The insn should satisfy the constraints of alternative 4. Well, not really. For the insn to match alternative 4 the pattern should be non-canonical. The last sentence in GCC Internals' canonicalization rules says: "Further canonicalization rules are defined in the function commutative_operand_ precedence in ‘gcc/rtlanal.c’" Unfortunately, commutative_operand_precendence() at the moment clearly states that a pointer (being that a MEM or a REG) has precedence over other RTX_OBJs. It is absolutely unclear to me why a pointer should have precedence over, say, multiplication or anything else (and "yes", I've read PR28690). With my target-independent hat on, I would remove that PPC-specific hunk from commutative_operand_precendence() or, if that is really that important to PPC, add a new target hook so that different targets can enjoy privilege of defining that to whatever they seem fit. Adding such an obscure canonicalization rule for all targets seems unjustified. I'd like to get some feedback on the above before I start implementing new target hook to make all targets happy. Peter, I'm CCing you as the author of the commutative_operand_precendence() piece to get your opinion on the above.
But % makes it commutative, no? So operand 2 matches operand 0, and operand 1 matches mSrIKLT.
(In reply to comment #9) > But % makes it commutative, no? Yes, but that only means that the operands can be swapped *if* swap_commutative_operands_p() returns true. Due to the funny precedence that does not happen. > So operand 2 matches operand 0, and operand 1 > matches mSrIKLT. Matching procedures do not take commutativeness into account. Part of the problem are optimizations using commutativeness during reload. One way to paper over the issue is to forbid such optimizations during reload, but that may worsen code. And this approach is not as clean as letting backends decide if their .md files can handle funny canonicalization rules.
> Yes, but that only means that the operands can be swapped *if* > swap_commutative_operands_p() returns true. Due to the funny precedence that > does not happen. > > Matching procedures do not take commutativeness into account. That would mean that PR28690 screwed up two-address machines big. In that case my opinion of the patch would change. Still, I don't think a target hook is the solution. Even if it adds hack over hack, having the "funny precedence" rules only before reload could be a solution. I'm pretty sure that we are talking about two different things. :-)
(In reply to comment #11) > Still, I don't think a target hook is the solution. Even if it adds hack over > hack, having the "funny precedence" rules only before reload could be a > solution. For the record, I consider the hook to be a feature, the one which backends can use to tweak code generation, not a hack. At the moment, the precedence which favors pointers is a hack, but we cannot remove it due to powerpc being a important platform. Also, favoring the pointers may be useful for other similar to ppc platforms even if we are not aware of them right now. That said, conditioning the precedence on (!reload_in_progress && !reload_completed) fixes the bug so I consider this to be the second best thing to a hook. It is not as good as a hook because there may be corner cases during reload when pointers should be favored on powerpc (multi-dimension array references, probably). On the other hand m68k need unrestricted (in the sense that all RTX_OBJ are the same) commutativeness during reload to avoid ICEs. So, conditioning pointer precedence on reload_in_progress has possible performance degradation on powerpc on one hand and ICE on m68k on the other. > I'm pretty sure that we are talking about two different things. :-) Err, I don't fully follow you here. What are the two different things?
Created attachment 18061 [details] Proposed patch Here is a patch moving precedence handling of pointers to powerpc backend.
GCC 4.3.4 is being released, adjusting target milestone.
There are several (4, I think) patches posted in gcc-patches@ for this bug. A reload/recog maintainer is needed to choose the most appropriate one.
There have been many patches posted, but most have caused serious performance degradations on power. However, the two latest patches to reload do not. They are: 1) http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00816.html 2) http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00823.html Maxim, have you tried either of these patches and do they work for you? Uli, can you please have a look at Richard's and Paolo's patches and does one or the other seem like a "better" fix?
I'll try the above two patches and will report in a couple of days.
(In reply to comment #16) > Uli, can you please have a look at Richard's and Paolo's patches and does one > or the other seem like a "better" fix? I've yet another suggestion :-) See my message at: http://gcc.gnu.org/ml/gcc-patches/2009-08/msg00254.html
Subject: Bug 37053 Author: uweigand Date: Mon Aug 10 15:34:09 2009 New Revision: 150626 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=150626 Log: PR target/37053 * reload1.c (reload_as_needed): Use cancel_changes to completely undo a failed replacement attempt. Modified: trunk/gcc/ChangeLog trunk/gcc/reload1.c
Fixed in 4.5.0.
Reopening since it is still broken on the other open branches.
GCC 4.3.5 is being released, adjusting target milestone.
Backported r150626 from Comment 19 and applied to Debian/m68k gcc-4.4 (native). Fixes this ICE when building libjpeg for me. No regression tests run, though.
You'd need also the patch for bug 41064.
*** Bug 40414 has been marked as a duplicate of this bug. ***
Probably, but not on m68k ;-)
Better: not for this testcase. We found it on CRIS, but the bug could really happen on any target.
OK, added.
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=602099#20 in case someone’s interested…
4.3 branch is being closed, moving to 4.4.7 target.
Fixed in 4.5+, 4.4 is no longer supported.