public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] Fix PR101188 wrong code from postreload
@ 2023-06-02  8:46 Georg-Johann Lay
  2023-06-03 15:53 ` Jeff Law
  0 siblings, 1 reply; 6+ messages in thread
From: Georg-Johann Lay @ 2023-06-02  8:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: ebotcazou

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;
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-10 16:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-02  8:46 [patch] Fix PR101188 wrong code from postreload Georg-Johann Lay
2023-06-03 15:53 ` Jeff Law
2023-06-03 18:38   ` Georg-Johann Lay
2023-06-10 16:45     ` Jeff Law
2023-06-05 15:06   ` Georg-Johann Lay
2023-06-05 15:55     ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).