| Message ID | 5523F167.3010606@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 07, 2015 at 11:01:59AM -0400, Vladimir Makarov wrote: > The following patch fixes > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65648 > > The patch was bootstrapped and tested on x86-64. > > I am lost to produce a test for the PR which can work on all arm > sub-targets and have no sub-target hardware to test it. > Therefore the patch does not contain the test. It would be nice if > somebody knowing arm well add a test for the PR. > > Committed as rev. 221901. > > 2015-04-07 Vladimir Makarov <vmakarov@redhat.com> > > PR target/65678 PR target/65648 instead? Jakub
On Tue, Apr 07, 2015 at 11:01:59AM -0400, Vladimir Makarov wrote: > 2015-04-07 Vladimir Makarov <vmakarov@redhat.com> > > PR target/65678 > * lra-remat.c (do_remat): Process input and non-input insn > registers separately. Don't have a quick access to arm box right now (without waiting for it to be installed etc.), but using a cross-compiler I can at least reproduce that your patch changes also: /* PR target/65648 */ int a = 0, *b = 0, c = 0; static int d = 0; short e = 1; static long long f = 0; long long *i = &f; unsigned char j = 0; __attribute__((noinline, noclone)) void foo (int x, int *y) { asm volatile ("" : : "r" (x), "r" (y) : "memory"); } __attribute__((noinline, noclone)) void bar (const char *x, long long y) { asm volatile ("" : : "r" (x), "r" (&y) : "memory"); if (y != 0) __builtin_abort (); } int main () { int k = 0; b = &k; j = (!a) - (c <= e); *i = j; foo (a, &k); bar ("", f); return 0; } so, if anybody can confirm this testcase aborts with -march=armv6-m -mthumb -Os before Vlad's patch and doesn't abort afterwards, perhaps just sticking that into gcc.c-torture/execute/pr65648.c would be sufficient. Or, if people don't regularly test with -march=armv6-m -mthumb combination, perhaps put it into gcc.c-torture/execute/pr65648.c as is and add another gcc.target/arm/pr65648.c testcase that will #include this one, and use the right dg-options / dg-skip-if or dg-require-effective-target etc. for it to trigger. As the testcase uses uninitialized r3 in the wrong case, I wonder if the usual _start initializes r3 to some value that triggers the abort without the fix; if not, perhaps it needs to be in another function and the caller should somehow attempt to set l3 somehow (pass arguments to another function etc.) to a value that will trigger the abort. Jakub
Index: lra-remat.c =================================================================== --- lra-remat.c (revision 221867) +++ lra-remat.c (working copy) @@ -1234,22 +1234,25 @@ do_remat (void) for (i = 0; i < nregs; i++) CLEAR_HARD_REG_BIT (live_hard_regs, hard_regno + i); } - else if (reg->type != OP_IN - && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL) + /* Process also hard regs (e.g. CC register) which are part + of insn definition. */ + for (reg = static_id->hard_regs; reg != NULL; reg = reg->next) + if (reg->type == OP_IN + && find_regno_note (insn, REG_DEAD, reg->regno) != NULL) + CLEAR_HARD_REG_BIT (live_hard_regs, reg->regno); + /* Inputs have been processed, now process outputs. */ + for (reg = id->regs; reg != NULL; reg = reg->next) + if (reg->type != OP_IN + && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL) { if ((hard_regno = get_hard_regs (reg, nregs)) < 0) continue; for (i = 0; i < nregs; i++) SET_HARD_REG_BIT (live_hard_regs, hard_regno + i); } - /* Process also hard regs (e.g. CC register) which are part - of insn definition. */ for (reg = static_id->hard_regs; reg != NULL; reg = reg->next) - if (reg->type == OP_IN - && find_regno_note (insn, REG_DEAD, reg->regno) != NULL) - CLEAR_HARD_REG_BIT (live_hard_regs, reg->regno); - else if (reg->type != OP_IN - && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL) + if (reg->type != OP_IN + && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL) SET_HARD_REG_BIT (live_hard_regs, reg->regno); } }
The following patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65648 The patch was bootstrapped and tested on x86-64. I am lost to produce a test for the PR which can work on all arm sub-targets and have no sub-target hardware to test it. Therefore the patch does not contain the test. It would be nice if somebody knowing arm well add a test for the PR. Committed as rev. 221901. 2015-04-07 Vladimir Makarov <vmakarov@redhat.com> PR target/65678 * lra-remat.c (do_remat): Process input and non-input insn registers separately.