On Sun, Oct 20, 2024 at 7:01 PM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> After r15-4503-g8d6d6d537fdc75, phiopt could move a trapping statement
> across another trapping statement causing the order to be incorrect (might
> happen
> with non-call exceptions).
> To prevent this, a trapping statement has to be at the end of the basic block
> to be allowed to be factored. This adds that check.
I don't think we have any way to preserve the order of IEEE traps
(with respect to
FENV accesses or with respect themselves) or of externally throwing non-calls
and we do not attempt to do so.
So I don't think this fix is necessary (unless we move a stmt that can trap
somewhere it wasn't executed before, of course)
Richard.
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/117235
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (is_factor_profitable): Return false
> when a trapping statement is not the last statement of the
> basic block.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/factor_op_phi-5.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> .../gcc.dg/tree-ssa/factor_op_phi-5.c | 24 +++++++++++++++++++
> gcc/tree-ssa-phiopt.cc | 6 +++++
> 2 files changed, 30 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-5.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-5.c
> b/gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-5.c
> new file mode 100644
> index 00000000000..ad4338f3c08
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/factor_op_phi-5.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -ftrapping-math -fdump-tree-phiopt1-details" } */
> +
> +/* PR tree-optimization/117235 */
> +
> +/* The convert between int to float should not be factored as the statement
> + could be moved across another trapping statement. */
> +int f(int a, float b, float c, float d, int x, int y)
> +{
> + float e;
> + float t;
> + if (a)
> + {
> + e = x;
> + t = c/d;
> + } else
> + {
> + e = y;
> + t = d/c;
> + }
> + return e + t;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "changed to factor operation out from "
> "phiopt1" } } */
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index 38cc8506d1f..6a89f57a41a 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -249,6 +249,12 @@ is_factor_profitable (gimple *def_stmt, basic_block
> merge, tree arg)
> if (gsi_end_p (gsi))
> return true;
>
> + /* If the defining statement could trap, then it is never
> + profitable to factor out the statement if not at the end
> + of the bb. */
> + if (gimple_could_trap_p (def_stmt))
> + return false;
> +
> /* Check if the uses of arg is dominated by merge block, this is a quick
> and
> rough estimate if arg is still alive at the merge bb. */
> /* FIXME: extend to a more complete live range detection. */
> --
> 2.43.0
>
