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:
> if (TREE_CODE (rhs) == SSA_NAME)
> {
> ...
> if (is_gimple_assign (def1)
> && ((bit_expression_code == BIT_AND_EXPR
> && gimple_assign_rhs_code (def1) == EQ_EXPR)
> || (bit_expression_code == BIT_IOR_EXPR
> && gimple_assign_rhs_code (def1) == NE_EXPR)))
> {
> ...
> *code = gimple_assign_rhs_code (def1);
> return true;
> ...
> }
> }
> return false;
The function operand_equal_for_value_replacement only passes the value
it receives from rhs_is_fed_for_value_replacement to the call site.
At the call site, the returned value initializes the variable equal_p.
Thus, the return value is simply passed from
rhs_is_fed_for_value_replacement to the variable equal_p, and it is
true only in the situation of BIT_AND_EXPR/EQ_EXPR or
BIT_IOR_EXPR/NE_EXPR because that is how it is defined in
rhs_is_fed_for_value_replacement.
The patch only sets equal_p to true for the combinations AND/EQ and
OR/NE. These are the only two situations when equal_p becomes true as
a result of the patch, and therefore, only in those situations, as a
consequence of the patch, does the line of code you quoted execute:
> if (equal_p || maybe_equal_p)
> {
> ...
> e = (code == NE_EXPR ? true_edge : false_edge);
> ...
> if (...)
> {
> ...
> if (equal_p)
> {
> ...
> return 2;
> }
> }
> else if (equal_p)
> {
> ...
> return 1;
> }
> }
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.
CONFIDENTIALITY: The contents of this e-mail are confidential and intended only
for the above addressee(s). If you are not the intended recipient, or the
person responsible for delivering it to the intended recipient, copying or
delivering it to anyone else or using it in any unauthorized manner is
prohibited and may be unlawful. If you receive this e-mail by mistake, please
notify the sender and the systems administrator at straym...@rt-rk.com
immediately.
