There is the following bug in postreload that can be traced back
to v5 at least:

In postreload.cc::reload_cse_move2add() there is a loop over all
insns.  If it encounters a SET, the next insn is analyzed if it
is a single_set.

After next has been analyzed, it continues with

      if (success)
        delete_insn (insn);
      changed |= success;
      insn = next; // This effectively skips analysis of next.
      move2add_record_mode (reg);
      reg_offset[regno]
        = trunc_int_for_mode (added_offset + base_offset,
                              mode);
      continue; // for continues with insn = NEXT_INSN (insn).

So it records the effect of next, but not the clobbers that
next might have.  This is a problem if next clobbers a GPR
like it can happen for avr.  What then can happen is that in a
later round, it may use a value from a (partially) clobbered reg.

The patch records the effects of potential clobbers.

Bootstrapped and reg-tested on x86_64.  Also tested on avr where
the bug popped up.  The testcase discriminates on avr, and for now
I am not aware of any other target that's affected by the bug.

The change is not intrusive and fixes wrong code, so I'd like
to backport it.

Ok to apply?

Johann

rtl-optimization/101188: Don't bypass clobbers of some insns that are
optimized or are optimization candidates.

gcc/
        PR rtl-optimization/101188
        * postreload.cc (reload_cse_move2add): Record clobbers of next
        insn using move2add_note_store.

gcc/testsuite/
        PR rtl-optimization/101188
        * gcc.c-torture/execute/pr101188.c: New test.


diff --git a/gcc/postreload.cc b/gcc/postreload.cc
index fb392651e1b..2de3e2ea780 100644
--- a/gcc/postreload.cc
+++ b/gcc/postreload.cc
@@ -2033,6 +2033,14 @@ reload_cse_move2add (rtx_insn *first)
                      if (success)
                        delete_insn (insn);
                      changed |= success;
+                     // By setting "insn = next" below, we are bypassing the
+                     // side-effects of next, see PR101188.  Do them by hand
+                     subrtx_iterator::array_type array;
+                     FOR_EACH_SUBRTX (iter, array, PATTERN (next), NONCONST)
+                       {
+                         if (GET_CODE (*iter) == CLOBBER)
+                           move2add_note_store (XEXP (*iter, 0), *iter, next);
+                       }
                      insn = next;
                      move2add_record_mode (reg);
                      reg_offset[regno]
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr101188.c b/gcc/testsuite/gcc.c-torture/execute/pr101188.c
new file mode 100644
index 00000000000..4817c69347c
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr101188.c
@@ -0,0 +1,59 @@
+typedef __UINT8_TYPE__ uint8_t;
+typedef __UINT16_TYPE__ uint16_t;
+
+typedef uint8_t (*fn1)(void *a);
+typedef void (*fn2)(void *a, int *arg);
+
+struct S
+{
+    uint8_t buffer[64];
+    uint16_t n;
+    fn2 f2;
+    void *a;
+    fn1 f1;
+};
+
+volatile uint16_t x;
+
+void __attribute__((__noinline__,__noclone__))
+foo (uint16_t n)
+{
+  x = n;
+}
+
+void __attribute__((__noinline__,__noclone__))
+testfn (struct S *self)
+{
+    int arg;
+
+    foo (self->n);
+    self->n++;
+    self->f2 (self->a, &arg);
+    self->buffer[0] = self->f1 (self->a);
+}
+
+static unsigned char myfn2_called = 0;
+
+static void
+myfn2 (void *a, int *arg)
+{
+  myfn2_called = 1;
+}
+
+static uint8_t
+myfn1 (void *a)
+{
+  return 0;
+}
+
+int main (void)
+{
+  struct S s;
+  s.n = 0;
+  s.f2 = myfn2;
+  s.f1 = myfn1;
+  testfn (&s);
+  if (myfn2_called != 1)
+    __builtin_abort();
+  return 0;
+}

Reply via email to