diff mbox series

[PR103028] test ifcvt trap_if seq more strictly after reload

Message ID oree6uhwlx.fsf@lxoliva.fsfla.org
State Committed
Commit daca416fc2816a5e481b26c8d2010127101d77ce
Headers show
Series [PR103028] test ifcvt trap_if seq more strictly after reload | expand

Commit Message

Alexandre Oliva Dec. 3, 2021, 9:21 a.m. UTC
When -fif-conversion2 is enabled, we attempt to replace conditional
branches around unconditional traps with conditional traps.  That
canonicalizes compares, which may change an immediate that barely fits
into one that doesn't.

The compare for the trap is first checked using the predicates of
cbranch predicates, and then, compare and conditional trap insns are
emitted and recognized.

In the failing s390x testcase, i <=u 0xffff_ffff is canonicalized into
i <u 0x1_0000_0000, and the latter immediate doesn't fit.  The insn
predicates (both cbranch and cmpdi_ccu) happily accept it, since the
register allocator has no trouble getting them into registers.  The
problem is that ifcvt2 runs after reload, so we recognize the compare
insn successfully, but later on we barf when we find that none of the
constraints fit.

This patch arranges for the trap_if-issuing bits in ifcvt to validate
post-reload insns using a stricter test that also checks that operands
fit the constraints.

Regstrapped on x86_64-linux-gnu.  Also verified that the testcase passes
on a cross to s390x-linux-gnu.  Ok to install?


for  gcc/ChangeLog

	PR rtl-optimization/103028
	* ifcvt.c (find_cond_trap): Validate new insns more strictly
	after reload.

for  gcc/testsuite/ChangeLog

	PR rtl-optimization/103028
	* gcc.dg/pr103028.c: New.
---
 gcc/ifcvt.c                     |    4 +++-
 gcc/testsuite/gcc.dg/pr103028.c |   21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr103028.c

Comments

Jeff Law Dec. 3, 2021, 2:56 p.m. UTC | #1
On 12/3/2021 2:21 AM, Alexandre Oliva via Gcc-patches wrote:
> When -fif-conversion2 is enabled, we attempt to replace conditional
> branches around unconditional traps with conditional traps.  That
> canonicalizes compares, which may change an immediate that barely fits
> into one that doesn't.
>
> The compare for the trap is first checked using the predicates of
> cbranch predicates, and then, compare and conditional trap insns are
> emitted and recognized.
>
> In the failing s390x testcase, i <=u 0xffff_ffff is canonicalized into
> i <u 0x1_0000_0000, and the latter immediate doesn't fit.  The insn
> predicates (both cbranch and cmpdi_ccu) happily accept it, since the
> register allocator has no trouble getting them into registers.  The
> problem is that ifcvt2 runs after reload, so we recognize the compare
> insn successfully, but later on we barf when we find that none of the
> constraints fit.
>
> This patch arranges for the trap_if-issuing bits in ifcvt to validate
> post-reload insns using a stricter test that also checks that operands
> fit the constraints.
>
> Regstrapped on x86_64-linux-gnu.  Also verified that the testcase passes
> on a cross to s390x-linux-gnu.  Ok to install?
>
>
> for  gcc/ChangeLog
>
> 	PR rtl-optimization/103028
> 	* ifcvt.c (find_cond_trap): Validate new insns more strictly
> 	after reload.
>
> for  gcc/testsuite/ChangeLog
>
> 	PR rtl-optimization/103028
> 	* gcc.dg/pr103028.c: New.
OK.  I wouldn't be surprised if there's other cases were we're using 
recog_memoized post-reload when we should be using valid_insn_p.

Jeff
diff mbox series

Patch

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 017944f4f79aa..b0052f6c5ced3 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -4726,7 +4726,9 @@  find_cond_trap (basic_block test_bb, edge then_edge, edge else_edge)
 
   /* If that results in an invalid insn, back out.  */
   for (rtx_insn *x = seq; x; x = NEXT_INSN (x))
-    if (recog_memoized (x) < 0)
+    if (reload_completed
+	? !valid_insn_p (x)
+	: recog_memoized (x) < 0)
       return FALSE;
 
   /* Emit the new insns before cond_earliest.  */
diff --git a/gcc/testsuite/gcc.dg/pr103028.c b/gcc/testsuite/gcc.dg/pr103028.c
new file mode 100644
index 0000000000000..df96c62ddd36d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr103028.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fif-conversion2 -Og" } */
+/* { dg-options "-fif-conversion2 -Og -march=z9-ec" { target { s390x-*-* } } } */
+
+unsigned char x;
+int foo(void)
+{
+  unsigned long long i = x;
+  i = i + 0x80000000;
+  unsigned long long t = 0xffffffff;
+
+  if (i > t) {
+    unsigned long long ii;
+    asm("":"=g"(ii):"0"(i));
+    if ((ii <= t))
+      __builtin_trap();
+    return x;
+  }
+
+ return 0;
+}