Re: [PR80693] drop value of parallel SETs dropped by combine

[Alexandre Oliva <aoliva at redhat dot com>]

On Jun  8, 2017, Segher Boessenkool <segher@kernel.crashing.org> wrote:

> Would it work to just have "else" instead if this "if"?  Or hrm, we'll
> need to kill the recorded reg_stat value in the last case before this
> as well?

The patch below (is this what you meant?) fixes the PR testcase, and the
new else block doesn't get exercised in an x86_64-linux-gnu bootstrap.
However, it seems to me that it might very well drop parallel SETs
without decrementing the sets count, though probably in cases in which
this won't matter.

How's this?  (I haven't run regression tests yet)


[PR80693] drop value of parallel SETs dropped by combine

When an insn used by combine has multiple SETs, only the non-REG_UNUSED
set is used: others will end up dropped on the floor.  We have to take
note of the dropped REG_UNUSED SETs, clearing their cached values, so
that, even if the REGs remain used (e.g. because they were referenced
in the used SET_SRC), we will not use properties of the latest value
as if they applied to the earlier one.

for  gcc/ChangeLog

	PR rtl-optimization/80693
	* combine.c (distribute_notes): Reset any REG_UNUSED REGs that
	are not set or referenced in i3.

for  gcc/testsuite/ChangeLog

	PR rtl-optimization/80693
	* gcc.dg/pr80693.c: New.
---
 gcc/combine.c                  |   19 +++++++++++++++++++
 gcc/testsuite/gcc.dg/pr80693.c |   26 ++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr80693.c

diff --git a/gcc/combine.c b/gcc/combine.c
index 2d49bc2..477010d 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -14087,6 +14087,25 @@ distribute_notes (rtx notes, rtx_insn *from_insn, rtx_insn *i3, rtx_insn *i2,
 	      PUT_REG_NOTE_KIND (note, REG_DEAD);
 	      place = i3;
 	    }
+
+	  /* If there were any parallel sets in FROM_INSN other than
+	     the one setting IDEST, it must be REG_UNUSED, otherwise
+	     we could not have used FROM_INSN in combine.  Since this
+	     combine attempt succeeded, we know this unused SET was
+	     dropped on the floor, because the insn was either deleted
+	     or created from a new pattern that does not use its
+	     SET_DEST.	We must forget whatever we knew about the
+	     value that was stored by that SET, since the prior value
+	     may still be present in IDEST's src expression or
+	     elsewhere, and we do not want to use properties of the
+	     dropped value as if they applied to the prior one when
+	     simplifying e.g. subsequent combine attempts.  */
+	  else
+	    {
+	      gcc_assert (REG_P (XEXP (note, 0)));
+	      record_value_for_reg (XEXP (note, 0), NULL, NULL_RTX);
+	      INC_REG_N_SETS (REGNO (XEXP (note, 0)), -1);
+	    }
 	  break;
 
 	case REG_EQUAL:
diff --git a/gcc/testsuite/gcc.dg/pr80693.c b/gcc/testsuite/gcc.dg/pr80693.c
new file mode 100644
index 0000000..aecddd0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr80693.c
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+/* { dg-options "-O -fno-tree-coalesce-vars" } */
+typedef unsigned char u8;
+typedef unsigned short u16;
+typedef unsigned u32;
+typedef unsigned long u64;
+
+static u64 __attribute__((noinline, noclone))
+foo(u8 u8_0, u16 u16_0, u32 u32_0, u64 u64_0,  u16 u16_1)
+{
+  u16_1 += 0x1051;
+  u16_1 &= 1;
+  u8_0 <<= u32_0 & 7;
+  u16_0 -= !u16_1;
+  u16_1 >>= ((u16)-u8_0 != 0xff);
+  return u8_0 + u16_0 + u64_0 + u16_1;
+}
+
+int
+main (void)
+{
+  u64 x = foo(1, 1, 0xffff, 0, 1);
+  if (x != 0x80)
+    __builtin_abort();
+  return 0;
+}


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer