On 7/29/2026 9:21 AM, Avinal Kumar wrote:
On Wed, Jul 29, 2026 at 6:38 PM Jeffrey Law
<[email protected]> wrote:
I also included some whitespace changes in some comments. Please let me know
if they should be a separate change.
In general formatting fixes should be distinct changes unless they're
directly in the space you're working. These cases touch independent
patterns so an independent patch would be better. This policy is mostly
to make it easier to review by avoiding unnecessary diffs. It's also
the case that many formatting changes can go in without review under our
trivial/obvious guidelines. But I don't think it's worth splitting
these out this time. This is mostly a note for future contributions.
Understood, I will keep that in mind. Thanks
It would initially seem this is redundant with this pattern earlier in
match.pd:
|/* (X | Y) == Y becomes (X & ~Y) == 0. */ (simplify (cmp:c (bit_ior:c
@0 @1) @1) (cmp (bit_and @0 (bit_not! @1)) { build_zero_cst (TREE_TYPE
(@0)); }))|
So the main question/concern I have is why didn't the existing earlier
rule in match.pd fire? Presumably the "!" modifier on the bit_not is
rejecting the rewritten pattern?
Overall it looks good, I just want to make sure we're not utilizing the
more general pattern for a good reason.
jeff
The existing rule does not fire for this case. Consider these examples:
_Bool f1(unsigned len) {
unsigned newlen = len | 4;
return newlen == len;
}
_Bool f2(unsigned len) {
return (len | 5) == 5;
}
_Bool f3(unsigned x, unsigned y){
return (x | y) == y;
}
_Bool f4(unsigned x, unsigned y){
return (x | ~y) == ~y;
}
The capture bind @1 to whatever appears on the both side of the
comparision. The replacemnet in the existing rule needs bit_not! @1,
so a match is rejected unless ~@1 simplifies. For constant (5 as in
f2) or already negated value (~y as in f4) the rule fires. But fails
when @1 is a plain variable.
Removing ! would produce (4 & ~len) == 0 in f1, this is 3 ops. The new
pattern produces (len & 4) == 0, which avoids NOT entirely.
I would also note that the comment in the existing rule may be
slightly misleading, since readers will expect every form of (X | Y)
== Y to become (X & ~Y) == 0, which is not the case. I can put the
clarification as a follow-up.
Thanks. I think we're OK. I somehow kept zeroing in on the constant
simplifying. But in the case where the term on both sides of the test
is an SSA_NAME and the constant is 2^n, then we can rewrite into the
form you're doing without needing any simplification to occur.
I've pushed this to the trunk.
jeff