phiopt: Remove unused statements in sequence before calling of phiopt_early_allow
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
Commit Message
Since match and simplify can add unused statements to the sequence
in some cases, we should remove them before call phiopt_early_allow.
This is needed for 2 future patches. One is to allowing of comparisons,
optionally with a cast and optional with a negative expression.
The other is about supporting a way to handling
cond_removal_in_builtin_zero_pattern in match and also adding a new match
pattern to fix PR 126035 (ctz split back into one).
Bootstrapped and tested on x86_64-linux-gnu.
gcc/ChangeLog:
* tree-ssa-phiopt.cc (remove_unused_stmts): New function.
(gimple_simplify_phiopt): Call remove_unused_stmts before
phiopt_early_allow.
Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
---
gcc/tree-ssa-phiopt.cc | 48 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
Comments
On 7/8/2026 2:19 AM, Andrew Pinski wrote:
> Since match and simplify can add unused statements to the sequence
> in some cases, we should remove them before call phiopt_early_allow.
> This is needed for 2 future patches. One is to allowing of comparisons,
> optionally with a cast and optional with a negative expression.
> The other is about supporting a way to handling
> cond_removal_in_builtin_zero_pattern in match and also adding a new match
> pattern to fix PR 126035 (ctz split back into one).
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (remove_unused_stmts): New function.
> (gimple_simplify_phiopt): Call remove_unused_stmts before
> phiopt_early_allow.
I'm torn. Not particularly happy with what appears to be a mini DCE
pass on the sequence. Presumably the motivation is to clean things up
in a way that makes phiopt_early_allow more likely to return true by
removing statements that are going to be zapped by DCE later and thus
shouldn't participate in phiopt_early_allow?
jeff
On Thu, Jul 9, 2026 at 8:23 AM Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
>
>
>
> On 7/8/2026 2:19 AM, Andrew Pinski wrote:
> > Since match and simplify can add unused statements to the sequence
> > in some cases, we should remove them before call phiopt_early_allow.
> > This is needed for 2 future patches. One is to allowing of comparisons,
> > optionally with a cast and optional with a negative expression.
> > The other is about supporting a way to handling
> > cond_removal_in_builtin_zero_pattern in match and also adding a new match
> > pattern to fix PR 126035 (ctz split back into one).
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-phiopt.cc (remove_unused_stmts): New function.
> > (gimple_simplify_phiopt): Call remove_unused_stmts before
> > phiopt_early_allow.
> I'm torn. Not particularly happy with what appears to be a mini DCE
> pass on the sequence. Presumably the motivation is to clean things up
> in a way that makes phiopt_early_allow more likely to return true by
> removing statements that are going to be zapped by DCE later and thus
> shouldn't participate in phiopt_early_allow?
Yes. While working on a different patch I noticed this. Let me find
the testcase again.
The testcase is:
```
bool f1(bool a, bool b)
{
bool c = !a;
bool d = c & b;
if (d) return 0;
bool e = c | b;
return e;
}
```
This will also be rejected by the current version of
phiopt_early_allow. But we (Kyrylo and I) are looking to change that.
With the above patch what we get back is:
```
phiopt match-simplify back:
_12 = ~b_11(D);
_7 = a_9(D) | _12;
_6 = a_9(D) ^ b_11(D);
_5 = a_9(D) == b_11(D);
result: _5
rejected because early
```
But this change we get:
```
phiopt match-simplify back:
_5 = a_9(D) == b_11(D);
result: _5
rejected because early
```
which should then be accepted with the other change we are working on.
This was just one testcase I noticed the issue but I have seen in others.
The other reason to remove the dead statements from the sequence is
dealing with my next patch (which is not related to early). Which
extends phiopt match connection to handle (semi) arbitrary middle bbs
which allows the removal of cond_removal_in_builtin_zero_pattern and
allow for some of the patches that Daniel has been working on and also
allow for matching things like the 2 halved version of ctz into one
ctz with just a match pattern rather than some manual matching code.
Basically removing the dead statements makes sure we are no longer
referencing the middle-bb in some/many cases.
Thanks,
Andrea
>
> jeff
On 7/9/2026 12:21 PM, Andrea Pinski wrote:
> On Thu, Jul 9, 2026 at 8:23 AM Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
>>
>>
>> On 7/8/2026 2:19 AM, Andrew Pinski wrote:
>>> Since match and simplify can add unused statements to the sequence
>>> in some cases, we should remove them before call phiopt_early_allow.
>>> This is needed for 2 future patches. One is to allowing of comparisons,
>>> optionally with a cast and optional with a negative expression.
>>> The other is about supporting a way to handling
>>> cond_removal_in_builtin_zero_pattern in match and also adding a new match
>>> pattern to fix PR 126035 (ctz split back into one).
>>>
>>> Bootstrapped and tested on x86_64-linux-gnu.
>>>
>>> gcc/ChangeLog:
>>>
>>> * tree-ssa-phiopt.cc (remove_unused_stmts): New function.
>>> (gimple_simplify_phiopt): Call remove_unused_stmts before
>>> phiopt_early_allow.
>> I'm torn. Not particularly happy with what appears to be a mini DCE
>> pass on the sequence. Presumably the motivation is to clean things up
>> in a way that makes phiopt_early_allow more likely to return true by
>> removing statements that are going to be zapped by DCE later and thus
>> shouldn't participate in phiopt_early_allow?
> Yes. While working on a different patch I noticed this. Let me find
> the testcase again.
>
> The testcase is:
> ```
> bool f1(bool a, bool b)
> {
> bool c = !a;
> bool d = c & b;
> if (d) return 0;
> bool e = c | b;
> return e;
> }
> ```
>
> This will also be rejected by the current version of
> phiopt_early_allow. But we (Kyrylo and I) are looking to change that.
> With the above patch what we get back is:
> ```
> phiopt match-simplify back:
> _12 = ~b_11(D);
> _7 = a_9(D) | _12;
> _6 = a_9(D) ^ b_11(D);
> _5 = a_9(D) == b_11(D);
> result: _5
> rejected because early
> ```
> But this change we get:
> ```
> phiopt match-simplify back:
> _5 = a_9(D) == b_11(D);
> result: _5
> rejected because early
> ```
> which should then be accepted with the other change we are working on.
>
> This was just one testcase I noticed the issue but I have seen in others.
> The other reason to remove the dead statements from the sequence is
> dealing with my next patch (which is not related to early). Which
> extends phiopt match connection to handle (semi) arbitrary middle bbs
> which allows the removal of cond_removal_in_builtin_zero_pattern and
> allow for some of the patches that Daniel has been working on and also
> allow for matching things like the 2 halved version of ctz into one
> ctz with just a match pattern rather than some manual matching code.
> Basically removing the dead statements makes sure we are no longer
> referencing the middle-bb in some/many cases.
Understood. And presumably the dead statements are unavoidable when
we're going through the simplification steps. I can see why it's
appealing even though I don't like what we have to do to get there. We
may be in a limited-enough environment where a trivial mini-DCE
implementation works sensibly -- things like volatiles, calls, loops and
such shouldn't be a concern here.
Presumably the ggc_free is meant to recycle the relevant nodes more
quickly since this is expected to happen reasonably often?
The LLM review was concerned about the unchecked call to
gimple_get_lhs. But in this context I think every statement we generate
is going to have an LHS. SO that's probably a non-issue. Do we have any
scenario where the LHS isn't going to be an SSA_NAME? I think we
largely filter out memory references to avoid vop updates and such.
I'm inclined to conditionally ACK on the assumption the LLM issues with
gimple_get_lhs and the assumption it always returns an SSA_NAME are
non-issues in this context, but I'd consider this somewhat
controversial, so let's give folks until COB Monday to object to the
basic idea.
jeff
On Thu, Jul 9, 2026 at 2:07 PM Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
>
>
>
> On 7/9/2026 12:21 PM, Andrea Pinski wrote:
> > On Thu, Jul 9, 2026 at 8:23 AM Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
> >>
> >>
> >> On 7/8/2026 2:19 AM, Andrew Pinski wrote:
> >>> Since match and simplify can add unused statements to the sequence
> >>> in some cases, we should remove them before call phiopt_early_allow.
> >>> This is needed for 2 future patches. One is to allowing of comparisons,
> >>> optionally with a cast and optional with a negative expression.
> >>> The other is about supporting a way to handling
> >>> cond_removal_in_builtin_zero_pattern in match and also adding a new match
> >>> pattern to fix PR 126035 (ctz split back into one).
> >>>
> >>> Bootstrapped and tested on x86_64-linux-gnu.
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> * tree-ssa-phiopt.cc (remove_unused_stmts): New function.
> >>> (gimple_simplify_phiopt): Call remove_unused_stmts before
> >>> phiopt_early_allow.
> >> I'm torn. Not particularly happy with what appears to be a mini DCE
> >> pass on the sequence. Presumably the motivation is to clean things up
> >> in a way that makes phiopt_early_allow more likely to return true by
> >> removing statements that are going to be zapped by DCE later and thus
> >> shouldn't participate in phiopt_early_allow?
> > Yes. While working on a different patch I noticed this. Let me find
> > the testcase again.
> >
> > The testcase is:
> > ```
> > bool f1(bool a, bool b)
> > {
> > bool c = !a;
> > bool d = c & b;
> > if (d) return 0;
> > bool e = c | b;
> > return e;
> > }
> > ```
> >
> > This will also be rejected by the current version of
> > phiopt_early_allow. But we (Kyrylo and I) are looking to change that.
> > With the above patch what we get back is:
> > ```
> > phiopt match-simplify back:
> > _12 = ~b_11(D);
> > _7 = a_9(D) | _12;
> > _6 = a_9(D) ^ b_11(D);
> > _5 = a_9(D) == b_11(D);
> > result: _5
> > rejected because early
> > ```
> > But this change we get:
> > ```
> > phiopt match-simplify back:
> > _5 = a_9(D) == b_11(D);
> > result: _5
> > rejected because early
> > ```
> > which should then be accepted with the other change we are working on.
> >
> > This was just one testcase I noticed the issue but I have seen in others.
> > The other reason to remove the dead statements from the sequence is
> > dealing with my next patch (which is not related to early). Which
> > extends phiopt match connection to handle (semi) arbitrary middle bbs
> > which allows the removal of cond_removal_in_builtin_zero_pattern and
> > allow for some of the patches that Daniel has been working on and also
> > allow for matching things like the 2 halved version of ctz into one
> > ctz with just a match pattern rather than some manual matching code.
> > Basically removing the dead statements makes sure we are no longer
> > referencing the middle-bb in some/many cases.
> Understood. And presumably the dead statements are unavoidable when
> we're going through the simplification steps. I can see why it's
> appealing even though I don't like what we have to do to get there. We
> may be in a limited-enough environment where a trivial mini-DCE
> implementation works sensibly -- things like volatiles, calls, loops and
> such shouldn't be a concern here.
>
> Presumably the ggc_free is meant to recycle the relevant nodes more
> quickly since this is expected to happen reasonably often?
Yes. It is similar to what is done in gimple_seq_discard already.
>
> The LLM review was concerned about the unchecked call to
> gimple_get_lhs. But in this context I think every statement we generate
> is going to have an LHS. SO that's probably a non-issue. Do we have any
> scenario where the LHS isn't going to be an SSA_NAME? I think we
> largely filter out memory references to avoid vop updates and such.
Note gimple_match_op should never have any memory references in it.
(if it does then it is a bug; I fixed one back during GCC 15;
r15-3052-gc7b76a076cb2c6)
So in this case, what is returned from match always goes though
maybe_push_res_to_seq to push things on this sequence that we are
going to do a mini-dce on.
maybe_push_res_to_seq will always produce either a normal
gimple_assign with no load/stores or a call which is always const (it
specifically rejects non-const internal/builtin functions; non
builtin/internal calls can't be produced).
It also rejects ssa names which have abnormal too.
So there will always be a LHS.
>
> I'm inclined to conditionally ACK on the assumption the LLM issues with
> gimple_get_lhs and the assumption it always returns an SSA_NAME are
> non-issues in this context, but I'd consider this somewhat
> controversial, so let's give folks until COB Monday to object to the
> basic idea.
>
> jeff
On Wed, Jul 8, 2026 at 10:20 AM Andrew Pinski
<andrew.pinski@oss.qualcomm.com> wrote:
>
> Since match and simplify can add unused statements to the sequence
> in some cases, we should remove them before call phiopt_early_allow.
> This is needed for 2 future patches. One is to allowing of comparisons,
> optionally with a cast and optional with a negative expression.
> The other is about supporting a way to handling
> cond_removal_in_builtin_zero_pattern in match and also adding a new match
> pattern to fix PR 126035 (ctz split back into one).
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (remove_unused_stmts): New function.
> (gimple_simplify_phiopt): Call remove_unused_stmts before
> phiopt_early_allow.
>
> Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> ---
> gcc/tree-ssa-phiopt.cc | 48 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index e12dc7a8b0c..fd173ec01f4 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -688,6 +688,52 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
> }
> }
>
> +/* Remove unused statements in SEQ. That are used in OP.
> + This is needed for the way resimplify/match and simplify
> + works as it will place some unneeded statements in the sequence
> + which can get in the way of handling phiopt_early_allow. */
> +
> +static void
> +remove_unused_stmts (gimple_seq *seq, const gimple_match_op &op)
> +{
> + if (*seq == nullptr)
> + return;
> +
> + /* Since the sequence of statements are not part of
> + a basic block yet, have to manually go through the operands
> + to mark the uses. */
> + auto_bitmap uses;
> + for (unsigned i = 0; i < op.num_ops; i++)
> + {
> + tree use = op.ops[i];
> + if (TREE_CODE (use) == SSA_NAME)
> + bitmap_set_bit (uses, SSA_NAME_VERSION (use));
> + }
> + gimple_stmt_iterator gsi = gsi_last (*seq);
> + while (!gsi_end_p (gsi))
> + {
> + gimple *s = *gsi;
> + tree lhs = gimple_get_lhs (s);
> + if (bitmap_bit_p (uses, SSA_NAME_VERSION (lhs)))
> + {
> + for (unsigned i = 0; i < gimple_num_ops (s); i++)
> + {
> + tree use = gimple_op (s, i);
> + if (TREE_CODE (use) == SSA_NAME)
> + bitmap_set_bit (uses, SSA_NAME_VERSION (use));
I think this doesn't work on REALPART_EXPR and friends,
nor on &a[i_3]. Not sure the latter will appear, but for sure
the former will.
Walking SSA operands w/o SSA operands is ... hard (you
might want to dig out removed vectorizer code that needed
to do this for pattern stmts).
> + }
> + gsi_prev (&gsi);
> + continue;
> + }
> + gsi_remove (&gsi, true);
> + release_defs (s);
> + ggc_free (s);
> + /* If this was the last statement, start over. */
> + if (gsi_end_p (gsi))
> + gsi = gsi_last (*seq);
> + }
> +}
> +
> /* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
> Return NULL if nothing can be simplified or the resulting simplified value
> with parts pushed if EARLY_P was true. Also rejects non allowed tree code
> @@ -729,6 +775,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
>
> if (op.resimplify (&seq1, follow_all_ssa_edges))
> {
> + remove_unused_stmts (&seq1, op);
> bool allowed = !early_p || phiopt_early_allow (seq1, op);
> tree result = maybe_push_res_to_seq (&op, &seq1);
> if (dump_file && (dump_flags & TDF_FOLDING))
> @@ -783,6 +830,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
>
> if (op1.resimplify (&seq1, follow_all_ssa_edges))
> {
> + remove_unused_stmts (&seq1, op1);
> bool allowed = !early_p || phiopt_early_allow (seq1, op1);
> tree result = maybe_push_res_to_seq (&op1, &seq1);
> if (dump_file && (dump_flags & TDF_FOLDING))
> --
> 2.43.0
>
On Fri, Jul 10, 2026 at 3:03 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Wed, Jul 8, 2026 at 10:20 AM Andrew Pinski
> <andrew.pinski@oss.qualcomm.com> wrote:
> >
> > Since match and simplify can add unused statements to the sequence
> > in some cases, we should remove them before call phiopt_early_allow.
> > This is needed for 2 future patches. One is to allowing of comparisons,
> > optionally with a cast and optional with a negative expression.
> > The other is about supporting a way to handling
> > cond_removal_in_builtin_zero_pattern in match and also adding a new match
> > pattern to fix PR 126035 (ctz split back into one).
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-phiopt.cc (remove_unused_stmts): New function.
> > (gimple_simplify_phiopt): Call remove_unused_stmts before
> > phiopt_early_allow.
> >
> > Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> > ---
> > gcc/tree-ssa-phiopt.cc | 48 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> > index e12dc7a8b0c..fd173ec01f4 100644
> > --- a/gcc/tree-ssa-phiopt.cc
> > +++ b/gcc/tree-ssa-phiopt.cc
> > @@ -688,6 +688,52 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
> > }
> > }
> >
> > +/* Remove unused statements in SEQ. That are used in OP.
> > + This is needed for the way resimplify/match and simplify
> > + works as it will place some unneeded statements in the sequence
> > + which can get in the way of handling phiopt_early_allow. */
> > +
> > +static void
> > +remove_unused_stmts (gimple_seq *seq, const gimple_match_op &op)
> > +{
> > + if (*seq == nullptr)
> > + return;
> > +
> > + /* Since the sequence of statements are not part of
> > + a basic block yet, have to manually go through the operands
> > + to mark the uses. */
> > + auto_bitmap uses;
> > + for (unsigned i = 0; i < op.num_ops; i++)
> > + {
> > + tree use = op.ops[i];
> > + if (TREE_CODE (use) == SSA_NAME)
> > + bitmap_set_bit (uses, SSA_NAME_VERSION (use));
> > + }
> > + gimple_stmt_iterator gsi = gsi_last (*seq);
> > + while (!gsi_end_p (gsi))
> > + {
> > + gimple *s = *gsi;
> > + tree lhs = gimple_get_lhs (s);
> > + if (bitmap_bit_p (uses, SSA_NAME_VERSION (lhs)))
> > + {
> > + for (unsigned i = 0; i < gimple_num_ops (s); i++)
> > + {
> > + tree use = gimple_op (s, i);
> > + if (TREE_CODE (use) == SSA_NAME)
> > + bitmap_set_bit (uses, SSA_NAME_VERSION (use));
>
> I think this doesn't work on REALPART_EXPR and friends,
> nor on &a[i_3]. Not sure the latter will appear, but for sure
> the former will.
>
> Walking SSA operands w/o SSA operands is ... hard (you
> might want to dig out removed vectorizer code that needed
> to do this for pattern stmts).
Maybe use operands_scanner from tree-ssa-operands.cc and
expose the parse_ssa_operands method?
> > + }
> > + gsi_prev (&gsi);
> > + continue;
> > + }
> > + gsi_remove (&gsi, true);
> > + release_defs (s);
> > + ggc_free (s);
> > + /* If this was the last statement, start over. */
> > + if (gsi_end_p (gsi))
> > + gsi = gsi_last (*seq);
> > + }
> > +}
> > +
> > /* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
> > Return NULL if nothing can be simplified or the resulting simplified value
> > with parts pushed if EARLY_P was true. Also rejects non allowed tree code
> > @@ -729,6 +775,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
> >
> > if (op.resimplify (&seq1, follow_all_ssa_edges))
> > {
> > + remove_unused_stmts (&seq1, op);
> > bool allowed = !early_p || phiopt_early_allow (seq1, op);
> > tree result = maybe_push_res_to_seq (&op, &seq1);
> > if (dump_file && (dump_flags & TDF_FOLDING))
> > @@ -783,6 +830,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
> >
> > if (op1.resimplify (&seq1, follow_all_ssa_edges))
> > {
> > + remove_unused_stmts (&seq1, op1);
> > bool allowed = !early_p || phiopt_early_allow (seq1, op1);
> > tree result = maybe_push_res_to_seq (&op1, &seq1);
> > if (dump_file && (dump_flags & TDF_FOLDING))
> > --
> > 2.43.0
> >
On Fri, Jul 10, 2026 at 6:03 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Wed, Jul 8, 2026 at 10:20 AM Andrew Pinski
> <andrew.pinski@oss.qualcomm.com> wrote:
> >
> > Since match and simplify can add unused statements to the sequence
> > in some cases, we should remove them before call phiopt_early_allow.
> > This is needed for 2 future patches. One is to allowing of comparisons,
> > optionally with a cast and optional with a negative expression.
> > The other is about supporting a way to handling
> > cond_removal_in_builtin_zero_pattern in match and also adding a new match
> > pattern to fix PR 126035 (ctz split back into one).
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-phiopt.cc (remove_unused_stmts): New function.
> > (gimple_simplify_phiopt): Call remove_unused_stmts before
> > phiopt_early_allow.
> >
> > Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> > ---
> > gcc/tree-ssa-phiopt.cc | 48 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> > index e12dc7a8b0c..fd173ec01f4 100644
> > --- a/gcc/tree-ssa-phiopt.cc
> > +++ b/gcc/tree-ssa-phiopt.cc
> > @@ -688,6 +688,52 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
> > }
> > }
> >
> > +/* Remove unused statements in SEQ. That are used in OP.
> > + This is needed for the way resimplify/match and simplify
> > + works as it will place some unneeded statements in the sequence
> > + which can get in the way of handling phiopt_early_allow. */
> > +
> > +static void
> > +remove_unused_stmts (gimple_seq *seq, const gimple_match_op &op)
> > +{
> > + if (*seq == nullptr)
> > + return;
> > +
> > + /* Since the sequence of statements are not part of
> > + a basic block yet, have to manually go through the operands
> > + to mark the uses. */
> > + auto_bitmap uses;
> > + for (unsigned i = 0; i < op.num_ops; i++)
> > + {
> > + tree use = op.ops[i];
> > + if (TREE_CODE (use) == SSA_NAME)
> > + bitmap_set_bit (uses, SSA_NAME_VERSION (use));
> > + }
> > + gimple_stmt_iterator gsi = gsi_last (*seq);
> > + while (!gsi_end_p (gsi))
> > + {
> > + gimple *s = *gsi;
> > + tree lhs = gimple_get_lhs (s);
> > + if (bitmap_bit_p (uses, SSA_NAME_VERSION (lhs)))
> > + {
> > + for (unsigned i = 0; i < gimple_num_ops (s); i++)
> > + {
> > + tree use = gimple_op (s, i);
> > + if (TREE_CODE (use) == SSA_NAME)
> > + bitmap_set_bit (uses, SSA_NAME_VERSION (use));
>
> I think this doesn't work on REALPART_EXPR and friends,
> nor on &a[i_3]. Not sure the latter will appear, but for sure
> the former will.
>
> Walking SSA operands w/o SSA operands is ... hard (you
> might want to dig out removed vectorizer code that needed
> to do this for pattern stmts).
Yes you are correct I didn't think of REALPART_EXPR; though I don't
think it shows up in any pattern used with cond currently which is why
I didn't see an issue here.
I don't think `&a[i_3]` will show up but yes it is never too safe to be sorry.
> Maybe use operands_scanner from tree-ssa-operands.cc and
> expose the parse_ssa_operands method?
Yes I will look into that when I get back from vacation.
>
> > + }
> > + gsi_prev (&gsi);
> > + continue;
> > + }
> > + gsi_remove (&gsi, true);
> > + release_defs (s);
> > + ggc_free (s);
> > + /* If this was the last statement, start over. */
> > + if (gsi_end_p (gsi))
> > + gsi = gsi_last (*seq);
> > + }
> > +}
> > +
> > /* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
> > Return NULL if nothing can be simplified or the resulting simplified value
> > with parts pushed if EARLY_P was true. Also rejects non allowed tree code
> > @@ -729,6 +775,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
> >
> > if (op.resimplify (&seq1, follow_all_ssa_edges))
> > {
> > + remove_unused_stmts (&seq1, op);
> > bool allowed = !early_p || phiopt_early_allow (seq1, op);
> > tree result = maybe_push_res_to_seq (&op, &seq1);
> > if (dump_file && (dump_flags & TDF_FOLDING))
> > @@ -783,6 +830,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
> >
> > if (op1.resimplify (&seq1, follow_all_ssa_edges))
> > {
> > + remove_unused_stmts (&seq1, op1);
> > bool allowed = !early_p || phiopt_early_allow (seq1, op1);
> > tree result = maybe_push_res_to_seq (&op1, &seq1);
> > if (dump_file && (dump_flags & TDF_FOLDING))
> > --
> > 2.43.0
> >
@@ -688,6 +688,52 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
}
}
+/* Remove unused statements in SEQ. That are used in OP.
+ This is needed for the way resimplify/match and simplify
+ works as it will place some unneeded statements in the sequence
+ which can get in the way of handling phiopt_early_allow. */
+
+static void
+remove_unused_stmts (gimple_seq *seq, const gimple_match_op &op)
+{
+ if (*seq == nullptr)
+ return;
+
+ /* Since the sequence of statements are not part of
+ a basic block yet, have to manually go through the operands
+ to mark the uses. */
+ auto_bitmap uses;
+ for (unsigned i = 0; i < op.num_ops; i++)
+ {
+ tree use = op.ops[i];
+ if (TREE_CODE (use) == SSA_NAME)
+ bitmap_set_bit (uses, SSA_NAME_VERSION (use));
+ }
+ gimple_stmt_iterator gsi = gsi_last (*seq);
+ while (!gsi_end_p (gsi))
+ {
+ gimple *s = *gsi;
+ tree lhs = gimple_get_lhs (s);
+ if (bitmap_bit_p (uses, SSA_NAME_VERSION (lhs)))
+ {
+ for (unsigned i = 0; i < gimple_num_ops (s); i++)
+ {
+ tree use = gimple_op (s, i);
+ if (TREE_CODE (use) == SSA_NAME)
+ bitmap_set_bit (uses, SSA_NAME_VERSION (use));
+ }
+ gsi_prev (&gsi);
+ continue;
+ }
+ gsi_remove (&gsi, true);
+ release_defs (s);
+ ggc_free (s);
+ /* If this was the last statement, start over. */
+ if (gsi_end_p (gsi))
+ gsi = gsi_last (*seq);
+ }
+}
+
/* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
Return NULL if nothing can be simplified or the resulting simplified value
with parts pushed if EARLY_P was true. Also rejects non allowed tree code
@@ -729,6 +775,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
if (op.resimplify (&seq1, follow_all_ssa_edges))
{
+ remove_unused_stmts (&seq1, op);
bool allowed = !early_p || phiopt_early_allow (seq1, op);
tree result = maybe_push_res_to_seq (&op, &seq1);
if (dump_file && (dump_flags & TDF_FOLDING))
@@ -783,6 +830,7 @@ gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt,
if (op1.resimplify (&seq1, follow_all_ssa_edges))
{
+ remove_unused_stmts (&seq1, op1);
bool allowed = !early_p || phiopt_early_allow (seq1, op1);
tree result = maybe_push_res_to_seq (&op1, &seq1);
if (dump_file && (dump_flags & TDF_FOLDING))