public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFA][Bug target/108892 ][13 regression] Force re-recognition after changing RTL structure of an insn
@ 2023-03-29 13:48 Jeff Law
  2023-04-05 14:21 ` Segher Boessenkool
  0 siblings, 1 reply; 7+ messages in thread
From: Jeff Law @ 2023-03-29 13:48 UTC (permalink / raw)
  To: gcc-patches; +Cc: Segher Boessenkool

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

So as mentioned in the PR the underlying issue here is combine changes 
the form of an existing insn, but fails to force re-recognition.  As a 
result other parts of the compiler blow up.



>                   /* Temporarily replace the set's source with the
>                      contents of the REG_EQUAL note.  The insn will
>                      be deleted or recognized by try_combine.  */
>                   rtx orig_src = SET_SRC (set); 
>                   rtx orig_dest = SET_DEST (set); 
>                   if (GET_CODE (SET_DEST (set)) == ZERO_EXTRACT)
>                     SET_DEST (set) = XEXP (SET_DEST (set), 0);
>                   SET_SRC (set) = note;
>                   i2mod = temp;
>                   i2mod_old_rhs = copy_rtx (orig_src);
>                   i2mod_new_rhs = copy_rtx (note);
>                   next = try_combine (insn, i2mod, NULL, NULL,
>                                       &new_direct_jump_p, 
>                                       last_combined_insn);
>                   i2mod = NULL;
>                   if (next)
>                     {
>                       statistics_counter_event (cfun, "insn-with-note combine", 1);
>                       goto retry;
>                     } 
>                   SET_SRC (set) = orig_src;
>                   SET_DEST (set) = orig_dest;


This code replaces the SET_SRC of an insn in the RTL stream with the 
contents of a REG_EQUAL note.  So given an insn like this:

> (insn 122 117 127 2 (set (reg:DI 157 [ _46 ])
>         (ior:DI (reg:DI 200)
>             (reg:DI 251))) "j.c":14:5 -1
>      (expr_list:REG_EQUAL (const_int 25769803782 [0x600000006])
>         (nil)))

It replaces the (ior ...) with a (const_int ...).  The resulting insn is 
passed to try_combine which will try to recognize it, then use it in a 
combination attempt.  Recognition succeeds with the special 
define_insn_and_split pattern in the risc-v backend resulting in:

> (insn 122 117 127 2 (set (reg:DI 157 [ _46 ])
>         (const_int 25769803782 [0x600000006])) "j.c":14:5 177 {*mvconst_internal}
>      (expr_list:REG_EQUAL (const_int 25769803782 [0x600000006])
>         (nil)))

This is as-expected.  Now assume we were unable to combine anything, so 
try_combine returns NULL_RTX.  The quoted code above restores SET_SRC 
(and SET_DEST) resulting in:

> (insn 122 117 127 2 (set (reg:DI 157 [ _46 ])
>         (ior:DI (reg:DI 200)
>             (reg:DI 251))) "j.c":14:5 177 {*mvconst_internal}
>      (expr_list:REG_EQUAL (const_int 25769803782 [0x600000006])
>         (nil)))


But this doesn't get re-recognized and we ICE later in LRA.

The fix is trivial, reset the INSN_CODE to force re-recognition in the 
case where try_combine fails.

Bootstrapped and regression tested on x86_64 and riscv.   OK for the trunk?

Jeff

[-- Attachment #2: P --]
[-- Type: text/plain, Size: 1353 bytes --]

gcc/
	* combine.cc (combine_instructions): Force re-recognition when
	potentially changing the underlying RTL structure of an insn.

gcc/testsuite/
	* gcc.c-torture/compile/pr108892.c: New test

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 053879500b7..22bf8e1ec89 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -1416,6 +1416,7 @@ combine_instructions (rtx_insn *f, unsigned int nregs)
 		      statistics_counter_event (cfun, "insn-with-note combine", 1);
 		      goto retry;
 		    }
+		  INSN_CODE (temp) = -1;
 		  SET_SRC (set) = orig_src;
 		  SET_DEST (set) = orig_dest;
 		}
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr108892.c b/gcc/testsuite/gcc.c-torture/compile/pr108892.c
new file mode 100644
index 00000000000..d7fecd54ecf
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr108892.c
@@ -0,0 +1,23 @@
+typedef char __attribute__((__vector_size__ (64))) U;
+typedef int __attribute__((__vector_size__ (64))) V;
+
+int g;
+U u;
+
+static inline __attribute__((__always_inline__)) void
+bar (short a, short b, V w)
+{
+  V v = __builtin_shufflevector ((V) { }, a % (0 != w), 17, 22, 20, 15,
+				 20, 23, 17, 20, 16, 21, 16, 19, 18, 14, 15,
+				 14) ^ b;
+  g *= __builtin_memcmp_eq (0, 0, 2);
+  v |= 6;
+  __builtin_ilogb (0);
+  u = (U) w + (U) v;
+}
+
+void
+foo (void)
+{
+  bar (5, 4, (V){30, 4, 1, 5, 6});
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-04-08 18:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-29 13:48 [RFA][Bug target/108892 ][13 regression] Force re-recognition after changing RTL structure of an insn Jeff Law
2023-04-05 14:21 ` Segher Boessenkool
2023-04-05 15:07   ` Jeff Law
2023-04-05 17:38     ` Segher Boessenkool
2023-04-05 17:43       ` Jeff Law
2023-04-05 19:02         ` Segher Boessenkool
2023-04-08 18:57           ` 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).