On 7/9/2026 12:21 PM, Andrea Pinski wrote:
On Thu, Jul 9, 2026 at 8:23 AM Jeffrey Law <[email protected]> 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
