On Thu, Aug 03, 2023 at 01:20:00 AM Jeff Law <jeffreya...@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
