On Jun 8, 2017, Segher Boessenkool <seg...@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