| Message ID | 20141203011104.GA10500@gate.crashing.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 02, 2014 at 07:11:04PM -0600, Segher Boessenkool wrote: > Here is the testcase. I cannot actually test it on an m68k build, should > really do something about that (I build lots of cross compilers but they > cannot run the testsuite). Does it look okay, can you test it yourself? > [I did of course test the generated asm is indeed okay; just cannot test > the dejagnu stuff]. Note to self: "make check-gcc RUNTESTFLAGS=m68k.exp=pr52714.c" and don't forget that _yet again_. Testcase fails without the patch, works with, installing. Thanks for all the reviewing, Segher
On 12/02/14 18:11, Segher Boessenkool wrote: > On Mon, Dec 01, 2014 at 10:39:58AM -0700, Jeff Law wrote: >> Also OK with a testcase. I'd recommend just using the one from 52714, >> make it m68k specific. Not sure if it's best to scan the assembly or >> .combine dump -- your call. > > Here is the testcase. I cannot actually test it on an m68k build, should > really do something about that (I build lots of cross compilers but they > cannot run the testsuite). Does it look okay, can you test it yourself? > [I did of course test the generated asm is indeed okay; just cannot test > the dejagnu stuff]. > > > Segher > > > 2014-12-02 Segher Boessenkool <segher@kernel.crashing.org> > > gcc/testsuite/ > PR rtl-optimization/52714 > * gcc.target/m68k/pr52714.c: New testcase. THanks. Based on a later message, you were able to do the testing. At some point during the gcc-5 release process, I'll bootstrap m68k via Aranym. Jeff
On Wed, Dec 3, 2014 at 1:54 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Tue, Dec 02, 2014 at 07:11:04PM -0600, Segher Boessenkool wrote: >> Here is the testcase. I cannot actually test it on an m68k build, should >> really do something about that (I build lots of cross compilers but they >> cannot run the testsuite). Does it look okay, can you test it yourself? >> [I did of course test the generated asm is indeed okay; just cannot test >> the dejagnu stuff]. > > Note to self: "make check-gcc RUNTESTFLAGS=m68k.exp=pr52714.c" and don't > forget that _yet again_. > > Testcase fails without the patch, works with, installing. > > Thanks for all the reviewing, > > > Segher Hi Segher, Does your series of patches affect PR62151? I am going to revisit that one to see if I can answer Jeff's questions there. Thanks, bin
On Wed, Dec 03, 2014 at 05:31:15PM +0800, Bin.Cheng wrote: > Does your series of patches affect PR62151? I am going to revisit > that one to see if I can answer Jeff's questions there. I don't think any of my recent patches affects it. r215789 does, however, so you'll need to revert that to show the problem again. To recap: We have five insns, call them A, B, C, D, E. A feeds B feeds C; C feeds both D and E. A and C set the same pseudo. D and E are simple moves. A+B+C would not combine because of a limitation that r215789 removed. Then combine tried combining A+B+C+D doing effectively the combination of A+B+C, and un-cse'ing the result to D. Then distribute_notes messes up with the REG_DEAD note of B (the result of A is dead); it takes it to mean the result of C is dead, but E still uses that. r215789 hides this in the cases where A+B+C in fact combines, which in practice is (nearly) all cases. Either we need to fix distribute_notes, or we should disallow A->B->C->D combos where the result of C is not dead at D, to fix this for real. I vote for the latter: 4-insn combine is expensive, and doesn't often improve anything, and it's even less likely to match anything in this case (which after all needs to keep the result of A+B+C around, even though that on its own did not combine!) Even if it matches, it in effect does an uncse which combine does not normally do; it would be better to do that in a separate pass (maybe piggyback it on fwprop, its sibling; or maybe do it separately. Something for the future, anyway). Segher
diff --git a/gcc/testsuite/gcc.target/m68k/pr52714.c b/gcc/testsuite/gcc.target/m68k/pr52714.c new file mode 100644 index 0000000..0a52a1d --- /dev/null +++ b/gcc/testsuite/gcc.target/m68k/pr52714.c @@ -0,0 +1,33 @@ +/* PR rtl-optimization/52714 + + Check that combine manages to remove the "stack == 0" test. + Without ICEing. */ + +/* { dg-do compile } */ +/* { dg-options "-O1" } */ + +int __re_compile_fastmap(unsigned char *p) +{ + unsigned char **stack; + unsigned size; + unsigned avail; + + stack = __builtin_alloca(5 * sizeof(unsigned char*)); + if (stack == 0) + return -2; + size = 5; + avail = 0; + + for (;;) { + switch (*p++) { + case 0: + if (avail == size) + return -2; + stack[avail++] = p; + } + } + + return 0; +} + +/* { dg-final { scan-assembler-not {\mtst\.l %sp\M} } } */