On 5/3/25 07:41, Richard Biener wrote:
On Sat, May 3, 2025 at 12:39 AM Andrew MacLeod <amacl...@redhat.com> wrote:
On trunk I'll eventually do something different.. but it will be more
invasive than I think is reasonable for a backport.
The problem in the PR is that there is a variable with a range and has a
bitmask attached to it. We often defer bitmask processing, the the
change which triggers this problem "improves" the range by applying the
bitmask when we call update_bitmask. (PR 119712)
The case in point is a range of 0, combined with a bitmask that says the
'1' bit must be on. This results in an UNDEFINED range since its
impossible. this is rarely a problem but this particular snippet of
code in IPA is tripping over it because it has checked for undefined,
and then created a new range by combining the [0, 0] and the bitmask,
which we turn into an UNDEFINED.. which it isn't expected. and then
it asks for the type of the range.
As Jakub points out in the PR, this is effectively unreachable code that
is being propagated. A harmless fix would be to check if the result of
applying the bitmask results in an UNDEFINED value and to simply
replace it with a VARYING value.
WE still reduce the testcase to "return 0" and no more failure.
bootstraps on -x86_64-pc-linux-gnu with no regressions.
If this is acceptable, I will push it to trunk, then also test/verify
for the GCC15 and 14(?) branches and check it in there.
LGTM. IPA CP might want to either avoid looking at the type
for UNDEFINED or track it separate from the value-range, not
sure where it looks at the type of a range.
Richard.
This is the patch applied to GCC 15.
Bootstraps on x86_64-pc-linux-gnu with no regressions. OK for the GCC
15 release branch?
Andrew
From e43f8d4b1e0317cd90d3f1da4c61993a0e2e2c60 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacl...@redhat.com>
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
