On 1/6/26 12:35, Andrew MacLeod wrote:
VRP1 is allowed to remove a __builtin_unreachable () if it determines that the other edge in the branch dominates all uses of all exports from the block.

ie

<bb 3>:
  x_8 = 1 << i_7;
  if (x_8 <= 0)               <<-- this is the branch in BB3
    goto <bb 4>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 4> :
  __builtin_unreachable ();

we can remove the branch  and edge to bb 4 if we can prove that there are no uses of x_8 or i_7 in or dominated by the edge 3->5.  We can then set the global ranges of i_7 and x_8 and move along.

This PR demonstrates that this normally works, but with rangers ability to recompute values, we also have to look at the dependencies of all the exports.

IN this case i_7 is defined:
 <bb 7> :
  # i_2 = PHI <i_7(5), n_4(D)(2), i_7(6)>
  i_7 = i_2 + -1;
  if (i_2 > 0)
    goto <bb 3>; [INV]
  else
    goto <bb 8>; [INV]

so althoug the uses of i_7 Are dominated by 3->5,  it does NOT dominate the use of i_2 in bb_7.   When early removal changes the global value of i_7, ranger happily recomputes i_2 in the branch and decides that if i_7 is now [0, +INF], i_2 must always be > 0 and removes the branch.

Which is clearly incorrect.  This patch teaches the early removal code that it can only remove the unreachable call if x_8, i_7 and ALL the dependencies used to calculate either name are ALL dominated by the edge.    This information is all trivially available in GORI, so its noly a minor tweak.

Performance impact over a build of GCC is minimal. a 0.03% slowdown in VRP.

In the PR I mentioned not removing it if any of the dependencies had more than a single use, but that turned out to be too limiting.  This solution works much better.

Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK?

Andrew

Patch applied to GCC 15.   OK for branch or not?

Andrew
From e43f8d4b1e0317cd90d3f1da4c61993a0e2e2c60 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <[email protected]>
Date: Fri, 2 May 2025 15:48:08 -0400
Subject: [PATCH] Allow IPA_CP to handle UNDEFINED as VARYING.

When applying a bitmask to reflect ranges, it is sometimes deferred and
this can result in an UNDEFINED result.  IPA is not expecting this, and
add a check for it, and convert to VARYING if encountered.

	PR tree-optimization/120048
	gcc/
	* ipa-cp.cc (ipcp_store_vr_results): Check for UNDEFINED.

	gcc/testsuite/
	* gcc.dg/pr120048.c: New.
---
 gcc/ipa-cp.cc                   | 10 ++++++++++
 gcc/testsuite/gcc.dg/pr120048.c | 12 ++++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr120048.c

diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index 806c2bdc97f..a8ff3c87073 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -6398,6 +6398,11 @@ ipcp_store_vr_results (void)
 						     TYPE_PRECISION (type),
 						     TYPE_SIGN (type)));
 		  tmp.update_bitmask (bm);
+		  // Reflecting the bitmask on the ranges can sometime
+		  // produce an UNDEFINED value if the the bitmask update
+		  // was previously deferred.  See PR 120048.
+		  if (tmp.undefined_p ())
+		    tmp.set_varying (type);
 		  ipa_vr vr (tmp);
 		  ts->m_vr->quick_push (vr);
 		}
@@ -6419,6 +6424,11 @@ ipcp_store_vr_results (void)
 						 TYPE_PRECISION (type),
 						 TYPE_SIGN (type)));
 	      tmp.update_bitmask (bm);
+	      // Reflecting the bitmask on the ranges can sometime
+	      // produce an UNDEFINED value if the the bitmask update
+	      // was previously deferred.  See PR 120048.
+	      if (tmp.undefined_p ())
+		tmp.set_varying (type);
 	      ipa_vr vr (tmp);
 	      ts->m_vr->quick_push (vr);
 	    }
diff --git a/gcc/testsuite/gcc.dg/pr120048.c b/gcc/testsuite/gcc.dg/pr120048.c
new file mode 100644
index 00000000000..6bb34b0e168
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr120048.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-vrp -fno-tree-fre" } */
+
+int a, b, c;
+static int d(short e) { return e || (a && e) ? 0 : a; }
+static void f(int e) {
+  if (!e) {
+    d(0);
+    b = d(e);
+  }
+}
+int main() { f(c | 1); }
-- 
2.45.0

Reply via email to