Hi Richard,
Thanks for the suggestion (see more below).
> > > > This patch resolves PR c/119651 and PR c/123716 P4 regressions.
> > > > Tested on x86_64-pc-linux-gnu with make bootstrap and make -k
> > > > check with no new regressions. Ok for mainline?
> > >
> > > The tree_nop_conversion_p change looks OK. I wonder where we build
> > > the CONVERT_EXPR with error operands though, ideally we'd not do
> > > this but build error_mark_node for itself.
I've now committed the independent tree_nop_conversion_p change to resolve
PR c/123716. Thanks. The remaining fold-const.cc changes are discussed below.
> > You've asked this question before (can we avoid building error_operand_p
> nodes?).
> > Here https://gcc.gnu.org/pipermail/gcc-patches/2024-April/650244.html
> > The problem is tree sharing (similar to RTL sharing in the RTL optimizers).
> > VAR_DECLs are conceptually placed in a symbol table, and these trees
> > are re-used in multiple expressions in a function/compilation unit.
> > The redeclaration of a variable with conflicting type, causes us to
> > set/update it's type to error_mark_node, poisoning not only future
> > uses, but all prior uses of that variable. This means that
> > problematic expressions are not error_operand_p at the time that they
> > are built, but "magically" become error_operands_p at a later point.
> >
> > This is a bit like changing the register number in an RTL pseudo
> > register, where all references to that pseudo change [during register
> > allocation].
> >
> > I completely agree it's ugly.
>
> Ah yeah, I now remember ...
>
> > One approach might be to consider redeclaration of variables a fatal
> > error, and refuse to attempt to continue compilation any further. The
> > alternative is the current whack-a-mole where we attempt to make the
> > middle-end's tree optimizers robust to VAR_DECL's with TREE_TYPE
> error_mark_node.
> > Automatic fuzz testing finds these ICEs faster than we can review their
> > fixes.
>
> Right.
>
> > > CASE_CONVERT:
> > > + if (TREE_TYPE (t) == error_mark_node
> > >
> > > this sub-check I'd check
> > >
> > > if (!error_operand_p (t))
> > >
> > > around the switch.
> > >
> > > + || TREE_TYPE (TREE_OPERAND (t, 0)) == error_mark_node)
> > > + break;
> >
> > Of course if we'd know that we had an error_operand_p at build time,
> > we'd have returned error_mark_node instead of constructing a
> > CASE_CONVERT.
>
> So the patch is OK with the above adjustment.
Checking through some related error_mark_node ICEs that have been closed as
duplicates, I came across PR c/123472 which reveals the version 1 patch (plus
the
suggested error_operand_p before the switch) is insufficient. The problem is
what
to return from tree_nonzero_bits when the type is error_mark node.
Unfortunately,
wide-int.cc doesn't permit a ~0 return value of infinite or undefined
precision. My
next attempt to use an arbitrary precision of 64 bits (consistent with host
wide int)
revealed that the recursive nature of tree_nonzero_bits expects a bitset result
with
a specific precision (to support bit_and/bit_ior operations). The solution is
to hoist
the suggested error_operand_p even earlier, before the recursion, after which
calls
to tree_nozero_bits can pass a precision, effectively TYPE_PRECISION (TREE_TYPE
(t)),
that can be used (in the default return case of -1) without requiring that
TREE_TYPE is (locally) valid. Hopefully, this all makes sense.
What do you think of this revised patch, which now resolves both
PR 119651 (as before) and (now also) PR 123472?
Tested on x86_64-pc-linux-gnu with make bootstrap and make -k check
with no new regressions. Ok for mainline?
2026-02-22 Roger Sayle <[email protected]>
gcc/ChangeLog
PR c/119651
PR c/123472
* fold-const.cc (tree_nonzero_bits): Rename the original as a
static function taking an additional precision parameter. Make
this implementation robust to error_mark_node. Preserve the
original API by checking for error_operand_p before invoking the
static helper function.
gcc/testsuite/ChangeLog
PR c/119651
PR c/123472
* gcc.dg/pr119651.c: New test case.
* gcc.dg/pr123472.c: Likewise.
Thanks in advance,
Roger
--
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 41681d38570..19f25d40d7f 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -16708,10 +16708,11 @@ c_getstr (tree str)
return getbyterep (str, NULL);
}
-/* Given a tree T, compute which bits in T may be nonzero. */
+/* Helper for tree_nonzero_bits. Given a tree T, compute which bits in T
+ may be nonzero, with precision PREC, the precision of T's type. */
-wide_int
-tree_nonzero_bits (const_tree t)
+static wide_int
+tree_nonzero_bits (const_tree t, unsigned prec)
{
switch (TREE_CODE (t))
{
@@ -16721,59 +16722,76 @@ tree_nonzero_bits (const_tree t)
return get_nonzero_bits (t);
case NON_LVALUE_EXPR:
case SAVE_EXPR:
- return tree_nonzero_bits (TREE_OPERAND (t, 0));
+ return tree_nonzero_bits (TREE_OPERAND (t, 0), prec);
case BIT_AND_EXPR:
- return wi::bit_and (tree_nonzero_bits (TREE_OPERAND (t, 0)),
- tree_nonzero_bits (TREE_OPERAND (t, 1)));
+ return wi::bit_and (tree_nonzero_bits (TREE_OPERAND (t, 0), prec),
+ tree_nonzero_bits (TREE_OPERAND (t, 1), prec));
case BIT_IOR_EXPR:
case BIT_XOR_EXPR:
- return wi::bit_or (tree_nonzero_bits (TREE_OPERAND (t, 0)),
- tree_nonzero_bits (TREE_OPERAND (t, 1)));
+ return wi::bit_or (tree_nonzero_bits (TREE_OPERAND (t, 0), prec),
+ tree_nonzero_bits (TREE_OPERAND (t, 1), prec));
case COND_EXPR:
- return wi::bit_or (tree_nonzero_bits (TREE_OPERAND (t, 1)),
- tree_nonzero_bits (TREE_OPERAND (t, 2)));
+ return wi::bit_or (tree_nonzero_bits (TREE_OPERAND (t, 1), prec),
+ tree_nonzero_bits (TREE_OPERAND (t, 2), prec));
CASE_CONVERT:
- return wide_int::from (tree_nonzero_bits (TREE_OPERAND (t, 0)),
- TYPE_PRECISION (TREE_TYPE (t)),
- TYPE_SIGN (TREE_TYPE (TREE_OPERAND (t, 0))));
+ if (TREE_TYPE (t) != error_mark_node
+ && !error_operand_p (TREE_OPERAND (t, 0)))
+ {
+ tree op0 = TREE_OPERAND (t, 0);
+ tree inner_type = TREE_TYPE (op0);
+ unsigned inner_prec = TYPE_PRECISION (inner_type);
+ return wide_int::from (tree_nonzero_bits (op0, inner_prec),
+ prec, TYPE_SIGN (inner_type));
+ }
+ break;
case PLUS_EXPR:
if (INTEGRAL_TYPE_P (TREE_TYPE (t)))
{
- wide_int nzbits1 = tree_nonzero_bits (TREE_OPERAND (t, 0));
- wide_int nzbits2 = tree_nonzero_bits (TREE_OPERAND (t, 1));
+ wide_int nzbits1 = tree_nonzero_bits (TREE_OPERAND (t, 0), prec);
+ wide_int nzbits2 = tree_nonzero_bits (TREE_OPERAND (t, 1), prec);
if (wi::bit_and (nzbits1, nzbits2) == 0)
return wi::bit_or (nzbits1, nzbits2);
}
break;
case LSHIFT_EXPR:
- if (TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST)
+ if (TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST
+ && TREE_TYPE (t) != error_mark_node)
{
tree type = TREE_TYPE (t);
- wide_int nzbits = tree_nonzero_bits (TREE_OPERAND (t, 0));
- wide_int arg1 = wi::to_wide (TREE_OPERAND (t, 1),
- TYPE_PRECISION (type));
+ wide_int nzbits = tree_nonzero_bits (TREE_OPERAND (t, 0), prec);
+ wide_int arg1 = wi::to_wide (TREE_OPERAND (t, 1), prec);
return wi::neg_p (arg1)
? wi::rshift (nzbits, -arg1, TYPE_SIGN (type))
: wi::lshift (nzbits, arg1);
}
break;
case RSHIFT_EXPR:
- if (TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST)
- {
+ if (TREE_CODE (TREE_OPERAND (t, 1)) == INTEGER_CST
+ && TREE_TYPE (t) != error_mark_node)
+ {
tree type = TREE_TYPE (t);
- wide_int nzbits = tree_nonzero_bits (TREE_OPERAND (t, 0));
- wide_int arg1 = wi::to_wide (TREE_OPERAND (t, 1),
- TYPE_PRECISION (type));
+ wide_int nzbits = tree_nonzero_bits (TREE_OPERAND (t, 0), prec);
+ wide_int arg1 = wi::to_wide (TREE_OPERAND (t, 1), prec);
return wi::neg_p (arg1)
? wi::lshift (nzbits, -arg1)
: wi::rshift (nzbits, arg1, TYPE_SIGN (type));
- }
+ }
break;
default:
break;
}
- return wi::shwi (-1, TYPE_PRECISION (TREE_TYPE (t)));
+ return wi::shwi (-1, prec);
+}
+
+/* Given a tree T, compute which bits in T may be nonzero. */
+
+wide_int
+tree_nonzero_bits (const_tree t)
+{
+ if (error_operand_p (t))
+ return wi::shwi (-1, 64);
+ return tree_nonzero_bits (t, TYPE_PRECISION (TREE_TYPE (t)));
}
/* Helper function for address compare simplifications in match.pd.
diff --git a/gcc/testsuite/gcc.dg/pr119651.c b/gcc/testsuite/gcc.dg/pr119651.c
new file mode 100644
index 00000000000..d89c3c2b8cf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr119651.c
@@ -0,0 +1,10 @@
+/* PR c/119651 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+int main() {
+ int r = 0;
+ while (r % 2 == 0) {
+ }
+ double r = (double) sizeof(int); /* { dg-error "conflicting type" } */
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pr123472.c b/gcc/testsuite/gcc.dg/pr123472.c
new file mode 100644
index 00000000000..2f690c03db9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr123472.c
@@ -0,0 +1,16 @@
+/* PR c/123472 */
+/* { dg-do compile } */
+/* { do-options "-O2" } */
+
+int a , b , c ;
+__attribute__ ( ( __noinline__ ) ) int fn1 ( ) {
+if ( ( b | ( a != ( a & c ) ) ) == 1 )
+__builtin_abort ( ) ;
+return 0 ;
+}
+int c ( void ) { /* { dg-error "redeclared as different kind" } */
+a = 5 ;
+c = 1 ; /* { dg-error "lvalue required" } */
+b = 6 ;
+return fn1 ( ) ;
+}
