Re: [committed][RISC-V] Fix 20010221-1.c with zicond

["Xiao Zeng" <zengxiao@eswincomputing.com>]

On Thu, Aug 03, 2023 at 01:20:00 AM  Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
>So we're being a bit too aggressive with the .opt zicond patterns.
>
>
>> (define_insn "*czero.eqz.<GPR:mode><X:mode>.opt1"
>>   [(set (match_operand:GPR 0 "register_operand"                   "=r")
>>         (if_then_else:GPR (eq (match_operand:X 1 "register_operand" "r")
>>                               (const_int 0))
>>                           (match_operand:GPR 2 "register_operand" "1")
>>                           (match_operand:GPR 3 "register_operand" "r")))]
>>   "(TARGET_ZICOND || 1) && rtx_equal_p (operands[1], operands[2])"
>>   "czero.eqz\t%0,%3,%1"
>> )
>>
>The RTL semantics here are op0 = (op1 == 0) ? op1 : op2.  That maps
>directly to czero.eqz.  ie, we select op1 when we know it's zero, op2
>otherwise.  So this pattern is fine.
>
>
>
>> (define_insn "*czero.eqz.<GPR:mode><X:mode>.opt2"
>>   [(set (match_operand:GPR 0 "register_operand"                   "=r")
>>         (if_then_else:GPR (eq (match_operand:X 1 "register_operand" "r")
>>                               (const_int 0))
>>                           (match_operand:GPR 2 "register_operand" "r")
>>                           (match_operand:GPR 3 "register_operand" "1")))]
>>   "(TARGET_ZICOND || 1) && rtx_equal_p (operands[1],  operands[3])"
>>   "czero.nez\t%0,%2,%1"
>> )
>
>The RTL semantics of this pattern are are: op0 = (op1 == 0) ? op2 : op1;
>
>That's not something that can be expressed by the zicond extension as it
>selects op1 if and only if op1 is not equal to zero.
>
>
>
>> (define_insn "*czero.nez.<GPR:mode><X:mode>.opt3"
>>   [(set (match_operand:GPR 0 "register_operand"                   "=r")
>>         (if_then_else:GPR (ne (match_operand:X 1 "register_operand" "r")
>>                               (const_int 0))
>>                           (match_operand:GPR 2 "register_operand" "r")
>>                           (match_operand:GPR 3 "register_operand" "1")))]
>>   "(TARGET_ZICOND || 1) && rtx_equal_p (operands[1], operands[3])"
>>   "czero.eqz\t%0,%2,%1"
>> )
>The RTL semantics of this pattern are op0 = (op1 != 0) ? op2 : op1.
>That maps to czero.nez.  But the output template uses czero.eqz.  Opps.
>
>> (define_insn "*czero.nez.<GPR:mode><X:mode>.opt4"
>>   [(set (match_operand:GPR 0 "register_operand"                   "=r")
>>         (if_then_else:GPR (ne (match_operand:X 1 "register_operand" "r")
>>                               (const_int 0))
>>                           (match_operand:GPR 2 "register_operand" "1")
>>                           (match_operand:GPR 3 "register_operand" "r")))]
>>   "(TARGET_ZICOND || 1) && rtx_equal_p (operands[1], operands[2])"
>>   "czero.nez\t%0,%3,%1"
>> )
>The RTL semantics of this pattern are op0 = (op1 != 0) ? op1 : op2 which
>obviously doesn't match to any zicond instruction as op1 is selected
>when it is not zero.
>
>
>So two of the patterns are just totally bogus as they are not
>implementable with zicond.  They are removed.  The asm template for the
>.opt3 pattern is fixed to use czero.nez and its name is changed to .opt2.
>
>This fixes the known issues with the zicond.md bits.  Onward to the rest
>of the expansion work :-)
>
>Committed to the trunk,
>
>jeff
>

Yes, two of these four optimization patterns are wrong.

In the wrong two optimization modes, I only considered the
case of satisfying the ELSE branch, but in fact, like the correct
two optimization modes, I should consider the case of satisfying
both the THAN and ELSE branches.

By the way, I was assigned other tasks during the week and
didn't have time to reply to emails, sorry.

Although I can't reply in time to the emails received from the
gcc community, I will definitely reply when I am free.

At the same time, I will improve my time management skills, keep
the same frequency with the community as much as possible, and
work better with everyone.

Thanks
Xiao Zeng