On Mon, May 5, 2025 at 12:00 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Mon, May 5, 2025 at 3:45 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
> >
> > Like the patch to phiopt (r15-4033-g1f619fe25925a5f7), this adds rewriting
> > of VCE to gimple_with_undefined_signed_overflow/rewrite_to_defined_overflow.
> > I have not seen a case yet for needing this rewrite but this step is needed
> > to use gimple_with_undefined_signed_overflow/rewrite_to_defined_overflow
> > from
> > phiopt.
>
> So what again was the "undefinedness" here? And why is this relevant
> for _signed overflow_? Wouldn't it be at least no problem to V_C_E
> a lower-precision value to a higher precision?
First has nothing to do with `signed integer overflow`, I should have
renamed the functions after this.
But the undefinedness is that on the condition VCE is valid since it
is known to be either 0/1.
But once you move the VCE to be unconditional, the value coming out of
the VCE might be out of range for the precision (in this case 0/1).
An example is from PR 116098:
```
<bb 4> [local count: 1073741824]:
if (value_7 == 0)
goto <bb 5>; [50.00%]
else
goto <bb 6>; [50.00%]
<bb 5> [local count: 536870912]:
<L2>:
_6 = VIEW_CONVERT_EXPR<_Bool>(value$8_8);
<bb 6> [local count: 1073741824]:
# _2 = PHI <_6(5), 1(4)>
```
And then we move the VCE from being conditional to being unconditional
and assume the resulting bool will be well defined.
Note I did found a testcase which shows the issue now on the trunk
(from GCC 14 onwards) without any new changes, PR 120122. This patch
will fix that case since we are pulling the VCE out of the loop but
not rewriting it.
>
> Maybe we should simply reject V_C_Es of non-mode-precision entities
> on GIMPLE, much like we reject BIT_FIELD_REFs of those.
I am all for that, SRA is what produces the majority of them.
>
> > Bootstrappd and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> > PR tree-optimization/116939
> > * gimple-fold.cc (gimple_with_undefined_signed_overflow): Return
> > true
> > for VCE with integral types.
> > (rewrite_to_defined_overflow): Handle VCE rewriting to a cast.
> >
> > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > ---
> > gcc/gimple-fold.cc | 30 ++++++++++++++++++++++++++++--
> > 1 file changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> > index c060ef81a42..a6a7fcbb8c1 100644
> > --- a/gcc/gimple-fold.cc
> > +++ b/gcc/gimple-fold.cc
> > @@ -10602,6 +10602,14 @@ gimple_with_undefined_signed_overflow (gimple
> > *stmt)
> > if (!INTEGRAL_TYPE_P (lhs_type)
> > && !POINTER_TYPE_P (lhs_type))
> > return false;
> > + tree rhs = gimple_assign_rhs1 (stmt);
> > + /* VCE from integral types to another integral types but with
> > + different precisions need to be changed into casts
> > + to be well defined. */
> > + if (gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR
> > + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (rhs, 0)))
> > + && is_gimple_val (TREE_OPERAND (rhs, 0)))
> > + return true;
> > if (!TYPE_OVERFLOW_UNDEFINED (lhs_type))
> > return false;
> > if (!arith_code_with_undefined_signed_overflow
> > @@ -10630,10 +10638,28 @@ rewrite_to_defined_overflow (gimple_stmt_iterator
> > *gsi, gimple *stmt,
> > "overflow ");
> > print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
> > }
> > -
> > + gimple_seq stmts = NULL;
> > + /* VCE from integral types to another integral types but with
> > + different precisions need to be changed into casts
> > + to be well defined. */
> > + if (gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR)
> > + {
> > + tree rhs = gimple_assign_rhs1 (stmt);
> > + tree new_rhs = TREE_OPERAND (rhs, 0);
> > + gcc_assert (is_gimple_val (new_rhs));
> > + gimple_assign_set_rhs_code (stmt, NOP_EXPR);
> > + gimple_assign_set_rhs1 (stmt, new_rhs);
> > + if (in_place)
> > + update_stmt (stmt);
> > + else
> > + {
> > + gimple_set_modified (stmt, true);
> > + gimple_seq_add_stmt (&stmts, stmt);
> > + }
> > + return stmts;
> > + }
> > tree lhs = gimple_assign_lhs (stmt);
> > tree type = unsigned_type_for (TREE_TYPE (lhs));
> > - gimple_seq stmts = NULL;
> > if (gimple_assign_rhs_code (stmt) == ABS_EXPR)
> > gimple_assign_set_rhs_code (stmt, ABSU_EXPR);
> > else
> > --
> > 2.34.1
> >
