| Message ID | 20170202093138.GF3731@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
On 02/02/2017 02:31 AM, Alan Modra wrote: > Revised patch that cures the lra related -m32 -Os regression too. > > The code that I'm patching here is changing a REG_EQUAL note to > REG_EQUIV, ie. asserting that the value of the reg is always the value > set by the current instruction. Which is not always true when the > insn is in a loop and the use of the reg happens before the def. Of > course that implies the value of the reg is initially undefined, so > it's not unreasonable to make the initial reg value the same. > Except when the code setting the initial value may trap, as it does in > the testcase. > > Bootstrap and regression test on x86_64-linux in progress. OK > assuming no regressions? > > I also took a look at gcc/*.o to see whether there were any code > quality regressions. No differences found except the expected change > in ira.o. > > PR rtl-optimization/79286 > * ira.c (update_equiv_regs): Do not create an equivalence for > mems that may trap. > testsuite/ > * gcc.c-torture/execute/pr79286.c: New. That seems pretty pessimistic -- do we have dominance information at this point? If so we could check that the assignment to the register dominates the use. If they are in the same block, then you have to look at LUIDs or somesuch. That would address the problem you're trying to solve without pessimizing the code. THe hell of it is I know I've poked at this problem in the past. jeff
On 02/02/2017 02:31 AM, Alan Modra wrote: > Revised patch that cures the lra related -m32 -Os regression too. > > The code that I'm patching here is changing a REG_EQUAL note to > REG_EQUIV, ie. asserting that the value of the reg is always the value > set by the current instruction. Which is not always true when the > insn is in a loop and the use of the reg happens before the def. Of > course that implies the value of the reg is initially undefined, so > it's not unreasonable to make the initial reg value the same. > Except when the code setting the initial value may trap, as it does in > the testcase. > > Bootstrap and regression test on x86_64-linux in progress. OK > assuming no regressions? > > I also took a look at gcc/*.o to see whether there were any code > quality regressions. No differences found except the expected change > in ira.o. > > PR rtl-optimization/79286 > * ira.c (update_equiv_regs): Do not create an equivalence for > mems that may trap. > testsuite/ > * gcc.c-torture/execute/pr79286.c: New. So I haven't been able to find where we've hashed through this issue before. My recollection was that if we encountered a use before the def that we would avoid recording the equivalence when we later see the REG_EQUAL note. That works within a BB. If the use/set are in different blocks then don't we just need to check that the set dominates the use? Jeff
diff --git a/gcc/ira.c b/gcc/ira.c index 96b4b62..8e79929 100644 --- a/gcc/ira.c +++ b/gcc/ira.c @@ -3500,7 +3500,9 @@ update_equiv_regs (void) /* If this register is known to be equal to a constant, record that it is always equivalent to the constant. */ if (DF_REG_DEF_COUNT (regno) == 1 - && note && ! rtx_varies_p (XEXP (note, 0), 0)) + && note + && !rtx_varies_p (XEXP (note, 0), 0) + && !may_trap_p (XEXP (note, 0))) { rtx note_value = XEXP (note, 0); remove_note (insn, note); diff --git a/gcc/testsuite/gcc.c-torture/execute/pr79286.c b/gcc/testsuite/gcc.c-torture/execute/pr79286.c new file mode 100644 index 0000000..e6d0e93 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr79286.c @@ -0,0 +1,15 @@ +int a = 0, c = 0; +static int d[][8] = {}; + +int main () +{ + int e; + for (int b = 0; b < 4; b++) + { + __builtin_printf ("%d\n", b, e); + while (a && c++) + e = d[300000000000000000][0]; + } + + return 0; +}