On 5/6/2026 2:59 AM, Richard Sandiford wrote:
+
+ /* Another variant seen on some backends, particularly those with
+ sub-word operations. For these cases we have to know there is no
+ carry from the PLUS into relevant bits. In practice that means
+ it's only valid for the uppermost bit. */
I think this is different enough that it's worth giving the expression
in the comment, i.e.
(ior (and A C1) (plus (and A C1) C2))
It took me a while to work that out from the condition :)
Quite fair. I actually had to remind myself how it worked after I'd
been away from it for a few months. Both are a good sign that a fresh
comment is warranted.
I wondered whether this one could/should be generalised to use
nonzero_bits, rather than checking specifically for (and A C1).
That is,
(ior X (plus X C))
(or the reverse, for simple X) is equivalent to (ior X C)
if the low bit of C is above the highest nonzero bit of X.
In the above example, that would give us:
(ior (and A C1) C2)
which admittedly is different from the expansion in the patch,
but is equally simple. This is one of those unfortunate cases where
there are many ways of writing the same thing...
The (ior X (plus X C)) with nonzero_bits thing works for xor or ior in
place of plus.
Seems like it's worth at least exploring to see if it's likely to trigger.
How about putting this in a for loop:
for (int i = 0; i < 2; ++i)
{
}
so that each transform needs to be written once? This seems like a
really large amount of code to cut-&-paste, and it would be easy for
mistakes to be made when editing in future.
Yea, I'd pondered creating a couple local variables and doing a
std::swap based on some initial shape queries to reduce duplication. In
this form I wouldn't even need to do form testing. Just std::swap on the
2nd iteration and be done with it.
Sorry for being awkward :)
Nothing to apologize for. I'm always happy to get your feedback.
Jeff
