[ Thanks for your patience. It's been a long month ;-) ]
On 11/4/24 6:23 AM, Jovan Vukic wrote:
On 11/02/24, Jeff Law wrote:
This is well understood. The key in my mind is that for AND we always
select the FALSE arm. For IOR we always select the TRUE arm.
Yes, I agree.
e = (code == NE_EXPR ? true_edge : false_edge);
If I understand everything correctly your assertion is that we'll only
get here for AND/EQ_EXPR and IOR/NE_EXPR. There's no way to get here
for AND/NE_EXPR or IOR/EQ_EXPR?
If we examine the patch step by step, we can see that the function
rhs_is_fed_for_value_replacement enters the if block exclusively for
the combinations BIT_AND_EXPR/EQ_EXPR and BIT_IOR_EXPR/NE_EXPR. It is
only at this point that it returns true and sets the value of *code. This is
evident in the code:
Thanks. I was just looking for a yes/no to verify that my understanding
was correct. It's been ~20 years since I looked at this code in any
significant way.
Your comments and re-reviewing the patch have addressed my concerns.
Also, Mr. Pinski left a comment
(https://gcc.gnu.org/pipermail/gcc-patches/2024-November/667258.html)
and offered some suggestions about the patch.
He also mentioned that he is working on integrating the affected code into
match-and-simplify, so the rewrite is on the way. We can either move forward
with this patch or stop it. Either way, I’m fine with any decision made.
I'd prefer to move forward. We're definitely moving a lot of
transformations into the match.pd framework and this one probably fits
in there reasonably well.
It'll make a bit more work for Andrew as part of his desire to move this
stuff into match.pd, but I don't see his work landing in this cycle and
he's indicated he doesn't have a problem with your patch going forward
at this time.
I'll push it into the tree after some additional light testing given
its' been a month since the last discussion on this patch.
jeff