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