| Message ID | 20241020170025.447681-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | phiopt: factor could move a trapping statement across other trapping statements [PR117235] | expand |
On Sun, Oct 20, 2024 at 7:01 PM Andrew Pinski <quic_apinski@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_apinski@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 >
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. */
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. 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_apinski@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