| Message ID | 20240819183714.3028693-2-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] phi-opt: Fix for non-const functions for factor_out_conditional_operation [PR 116409] | expand |
On Mon, Aug 19, 2024 at 8:38 PM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > After the conversion to use maybe_push_res_to_seq, sometimes (REALPART_EXPR > and IMAGPART_EXPR and VCE) the argument will not be a gimple value and > then phiopt here would create an invalid PHI. > Just add a check for gimple val is the way to fix this. > > Bootstrapped and tested on x86_64-linux-gnu. > > PR tree-optimization/116412 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (factor_out_conditional_operation): Make sure new_arg0 > and new_arg1 are both gimple values. Hmm, I think the bug is that gimple_extract_op produces REALPART_EXPR with *b_4(D) operand. The match machinery doesn't handle memory operands and we shouldn't feed it those - it might make wrong transforms. I think gimple_extract should return false for GIMPLE_SINGLE_RHS when the resulting op0 isn't a SSA name or a GIMPLE invariant. Note is_gimple_val would accept an aggregate variable which at least in the BIT_FIELD_REF case could appear, likewise for a V_C_E. Richard. > gcc/testsuite/ChangeLog: > > * gcc.dg/torture/pr116412-1.c: New test. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/testsuite/gcc.dg/torture/pr116412-1.c | 6 ++++++ > gcc/tree-ssa-phiopt.cc | 4 ++++ > 2 files changed, 10 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr116412-1.c > > diff --git a/gcc/testsuite/gcc.dg/torture/pr116412-1.c b/gcc/testsuite/gcc.dg/torture/pr116412-1.c > new file mode 100644 > index 00000000000..3bc26ecd8b8 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr116412-1.c > @@ -0,0 +1,6 @@ > +/* { dg-do compile } */ > +double f(_Complex double a, _Complex double *b, int c) > +{ > + if (c) return __real__ a; > + return __real__ *b; > +} > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index 770f3629fe1..be95798a065 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -368,6 +368,10 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, > if (!types_compatible_p (TREE_TYPE (new_arg0), TREE_TYPE (new_arg1))) > return NULL; > > + /* The new args need to be both gimple values. */ > + if (!is_gimple_val (new_arg0) || !is_gimple_val (new_arg1)) > + return NULL; > + > /* Function calls can only be const or an internal function > as maybe_push_res_to_seq only handles those currently. */ > if (!arg0_op.code.is_tree_code ()) > -- > 2.43.0 >
diff --git a/gcc/testsuite/gcc.dg/torture/pr116412-1.c b/gcc/testsuite/gcc.dg/torture/pr116412-1.c new file mode 100644 index 00000000000..3bc26ecd8b8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr116412-1.c @@ -0,0 +1,6 @@ +/* { dg-do compile } */ +double f(_Complex double a, _Complex double *b, int c) +{ + if (c) return __real__ a; + return __real__ *b; +} diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index 770f3629fe1..be95798a065 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -368,6 +368,10 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, if (!types_compatible_p (TREE_TYPE (new_arg0), TREE_TYPE (new_arg1))) return NULL; + /* The new args need to be both gimple values. */ + if (!is_gimple_val (new_arg0) || !is_gimple_val (new_arg1)) + return NULL; + /* Function calls can only be const or an internal function as maybe_push_res_to_seq only handles those currently. */ if (!arg0_op.code.is_tree_code ())
After the conversion to use maybe_push_res_to_seq, sometimes (REALPART_EXPR and IMAGPART_EXPR and VCE) the argument will not be a gimple value and then phiopt here would create an invalid PHI. Just add a check for gimple val is the way to fix this. Bootstrapped and tested on x86_64-linux-gnu. PR tree-optimization/116412 gcc/ChangeLog: * tree-ssa-phiopt.cc (factor_out_conditional_operation): Make sure new_arg0 and new_arg1 are both gimple values. gcc/testsuite/ChangeLog: * gcc.dg/torture/pr116412-1.c: New test. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/testsuite/gcc.dg/torture/pr116412-1.c | 6 ++++++ gcc/tree-ssa-phiopt.cc | 4 ++++ 2 files changed, 10 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/torture/pr116412-1.c