* [committed][RISC-V] Fix 20010221-1.c with zicond
@ 2023-08-02 17:20 Jeff Law
2023-08-04 9:29 ` Xiao Zeng
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2023-08-02 17:20 UTC (permalink / raw)
To: gcc-patches; +Cc: Xiao Zeng
[-- Attachment #1: Type: text/plain, Size: 3096 bytes --]
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
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 5356 bytes --]
commit 1d5bc3285e8a115538442dc2aaa34d2b509e1f6e
Author: Jeff Law <jlaw@ventanamicro.com>
Date: Wed Aug 2 13:16:23 2023 -0400
[committed][RISC-V] Fix 20010221-1.c with zicond
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.
gcc/
* config/riscv/zicond.md: Remove incorrect zicond patterns and
renumber/rename them.
(zero.nez.<GPR:MODE><X:mode>.opt2): Fix output string.
diff --git a/gcc/config/riscv/zicond.md b/gcc/config/riscv/zicond.md
index 8f24b3a1690..25f21d33487 100644
--- a/gcc/config/riscv/zicond.md
+++ b/gcc/config/riscv/zicond.md
@@ -53,32 +53,12 @@ (define_insn "*czero.eqz.<GPR:mode><X:mode>.opt1"
"czero.eqz\t%0,%3,%1"
)
-(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 && rtx_equal_p (operands[1], operands[3])"
- "czero.nez\t%0,%2,%1"
-)
-
-(define_insn "*czero.nez.<GPR:mode><X:mode>.opt3"
+(define_insn "*czero.nez.<GPR:mode><X:mode>.opt2"
[(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 && rtx_equal_p (operands[1], operands[3])"
- "czero.eqz\t%0,%2,%1"
-)
-
-(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 && rtx_equal_p (operands[1], operands[2])"
- "czero.nez\t%0,%3,%1"
+ "czero.nez\t%0,%2,%1"
)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed][RISC-V] Fix 20010221-1.c with zicond
2023-08-02 17:20 [committed][RISC-V] Fix 20010221-1.c with zicond Jeff Law
@ 2023-08-04 9:29 ` Xiao Zeng
2023-08-04 19:50 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Zeng @ 2023-08-04 9:29 UTC (permalink / raw)
To: jeffreyalaw, gcc-patches; +Cc: gaofei
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed][RISC-V] Fix 20010221-1.c with zicond
2023-08-04 9:29 ` Xiao Zeng
@ 2023-08-04 19:50 ` Jeff Law
2023-08-08 21:52 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2023-08-04 19:50 UTC (permalink / raw)
To: Xiao Zeng, gcc-patches; +Cc: gaofei
On 8/4/23 03:29, Xiao Zeng wrote:
> On Thu, Aug 03, 2023 at 01:20:00 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
> 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.
It happens -- we all make mistakes. FWIW I didn't spot it during the
review either.
>
> By the way, I was assigned other tasks during the week and
> didn't have time to reply to emails, sorry.
No worries. I'm trying to keep this moving because we have multiple
submissions from different authors in this space as well as bits
internal to Ventana. That's a recipe for a messy integration phase if
it's not well managed.
It's also something I kept meaning to resolve and your submission just
gave me the proper motivation to move zicond forward. The target
specific bits you did lined up perfectly with the community feedback on
the original VRULL implementation as well as the direction Ventana had
taken on our internal tree.
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed][RISC-V] Fix 20010221-1.c with zicond
2023-08-04 19:50 ` Jeff Law
@ 2023-08-08 21:52 ` Maciej W. Rozycki
2023-08-08 22:45 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2023-08-08 21:52 UTC (permalink / raw)
To: Jeff Law; +Cc: Xiao Zeng, gcc-patches, gaofei
On Fri, 4 Aug 2023, Jeff Law via Gcc-patches wrote:
> It's also something I kept meaning to resolve and your submission just gave me
> the proper motivation to move zicond forward. The target specific bits you
> did lined up perfectly with the community feedback on the original VRULL
> implementation as well as the direction Ventana had taken on our internal
> tree.
I wonder however why do we need so much more code, including the middle
end too, to support this ISA extension than we do for the very same set of
MIPSr6 instructions under ISA_HAS_SEL, hmm...
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed][RISC-V] Fix 20010221-1.c with zicond
2023-08-08 21:52 ` Maciej W. Rozycki
@ 2023-08-08 22:45 ` Jeff Law
2023-08-21 13:33 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2023-08-08 22:45 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Xiao Zeng, gcc-patches, gaofei
On 8/8/23 15:52, Maciej W. Rozycki wrote:
> On Fri, 4 Aug 2023, Jeff Law via Gcc-patches wrote:
>
>> It's also something I kept meaning to resolve and your submission just gave me
>> the proper motivation to move zicond forward. The target specific bits you
>> did lined up perfectly with the community feedback on the original VRULL
>> implementation as well as the direction Ventana had taken on our internal
>> tree.
>
> I wonder however why do we need so much more code, including the middle
> end too, to support this ISA extension than we do for the very same set of
> MIPSr6 instructions under ISA_HAS_SEL, hmm...
Because it doesn't handle as many cases as we're handling in the RISC-V
port.
I'd bet if you take Xiao's testcases and run them on a mips cross many,
if not most, won't optimize down into the mips equivalents.
One such example would be
(set (target)
(if_then_else (eq (reg A) (const_int 0))
(reg A)
(reg B)))
This is just one example obviously, but there are others.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed][RISC-V] Fix 20010221-1.c with zicond
2023-08-08 22:45 ` Jeff Law
@ 2023-08-21 13:33 ` Maciej W. Rozycki
0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2023-08-21 13:33 UTC (permalink / raw)
To: Jeff Law; +Cc: Xiao Zeng, gcc-patches, gaofei
On Tue, 8 Aug 2023, Jeff Law wrote:
> > I wonder however why do we need so much more code, including the middle
> > end too, to support this ISA extension than we do for the very same set of
> > MIPSr6 instructions under ISA_HAS_SEL, hmm...
> Because it doesn't handle as many cases as we're handling in the RISC-V port.
>
> I'd bet if you take Xiao's testcases and run them on a mips cross many, if not
> most, won't optimize down into the mips equivalents.
Actually most do, except for the arithmetic ones, so good point here,
thanks. As this is in the middle end it would be good to expand coverage
for the relevant non-RISC-V ports, perhaps in the same commit, or maybe
better yet, by committing the middle-end piece separately, immediately
followed by per-port individual testsuite updates.
Of the non-arithmetic ones, interestingly enough, MOVN and SELEQZ are
correctly produced with MIPS IV and MIPS64r6 compilation respectively,
however MOVZ and SELNEZ are missed in a couple of cases in favour to a
branched sequence which, given the complete symmetry of the operations,
suggests a silly bug in the backend somewhere.
> One such example would be
>
> (set (target)
> (if_then_else (eq (reg A) (const_int 0))
> (reg A)
> (reg B)))
>
> This is just one example obviously, but there are others.
This I believe corresponds to `primitiveSemantics_06' and it works with
the MIPS IV ISA producing MOVZ, but fails with the MIPS64r6 ISA where
SELNEZ is expected. Since the MIPS64r6 machine description pretends it
has a full conditional-move machine operation and emulates it via an RTL
expander with SELEQZ and SELNEZ combined with OR as required I still think
this particular expression is supposed to work with our tree without the
changes and it's probably due to a bug in the backend too, possibly one
considered in the previous paragraph.
To double-check the plausibility of the hypothesis I have then tried Xiao
Zeng's <https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624907.html>
proposed patch, but it has caused an ICE:
during RTL pass: ce1
src/gcc/gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_reg_return_imm_reg.c: In function 'conditionalArithmetic_compare_reg_return_imm_reg_08':
src/gcc/gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_reg_return_imm_reg.c:85:1: internal compiler error: in gen_rtx_SUBREG, at emit-rtl.cc:1022
85 | }
| ^
As this was with GCC 12.0.1 20220404 (one I had handy, so quick to check)
I chose to retry with the top of the tree, i.e. 14.0.0 20230820. But the
ICE is still there:
during RTL pass: ce1
src/gcc/gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_reg_return_imm_reg.c: In function 'conditionalArithmetic_compare_reg_return_imm_reg_08':
src/gcc/gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_reg_return_imm_reg.c:85:1: internal compiler error: in gen_rtx_SUBREG, at emit-rtl.cc:1031
85 | }
| ^
And furthermore many of the test cases does not produce any of the
conditional moves anymore (whether with or without Xiao Zeng's patch).
This is with a `mips64-linux-gnu', `--with-abi=64' cross-compiler and
compilations made with `-mips4' and `-mips64r6' as appropriate.
E.g. with GCC 12:
$ grep -c movn zicond-*-mips4.s
zicond-conditionalArithmetic_compare_0_return_imm_reg-mips4.s:0
zicond-conditionalArithmetic_compare_0_return_reg_reg-mips4.s:0
zicond-conditionalArithmetic_compare_imm_return_imm_reg-mips4.s:0
zicond-conditionalArithmetic_compare_imm_return_reg_reg-mips4.s:0
zicond-conditionalArithmetic_compare_reg_return_imm_reg-mips4.s:0
zicond-conditionalArithmetic_compare_reg_return_reg_reg-mips4.s:0
zicond-primitiveSemantics-mips4.s:6
zicond-primitiveSemantics_compare_imm-mips4.s:6
zicond-primitiveSemantics_compare_imm_return_0_imm-mips4.s:6
zicond-primitiveSemantics_compare_imm_return_imm_imm-mips4.s:6
zicond-primitiveSemantics_compare_imm_return_imm_reg-mips4.s:6
zicond-primitiveSemantics_compare_imm_return_reg_reg-mips4.s:6
zicond-primitiveSemantics_compare_reg-mips4.s:6
zicond-primitiveSemantics_compare_reg_return_0_imm-mips4.s:6
zicond-primitiveSemantics_compare_reg_return_imm_imm-mips4.s:6
zicond-primitiveSemantics_compare_reg_return_imm_reg-mips4.s:6
zicond-primitiveSemantics_compare_reg_return_reg_reg-mips4.s:6
zicond-primitiveSemantics_return_0_imm-mips4.s:6
zicond-primitiveSemantics_return_imm_imm-mips4.s:6
zicond-primitiveSemantics_return_imm_reg-mips4.s:6
zicond-primitiveSemantics_return_reg_reg-mips4.s:6
$ grep -c seleqz zicond-*-mips64r6.s
zicond-conditionalArithmetic_compare_0_return_imm_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_0_return_reg_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_imm_return_imm_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_imm_return_reg_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_reg_return_imm_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_reg_return_reg_reg-mips64r6.s:0
zicond-primitiveSemantics-mips64r6.s:6
zicond-primitiveSemantics_compare_imm-mips64r6.s:6
zicond-primitiveSemantics_compare_imm_return_0_imm-mips64r6.s:6
zicond-primitiveSemantics_compare_imm_return_imm_imm-mips64r6.s:12
zicond-primitiveSemantics_compare_imm_return_imm_reg-mips64r6.s:12
zicond-primitiveSemantics_compare_imm_return_reg_reg-mips64r6.s:6
zicond-primitiveSemantics_compare_reg-mips64r6.s:6
zicond-primitiveSemantics_compare_reg_return_0_imm-mips64r6.s:6
zicond-primitiveSemantics_compare_reg_return_imm_imm-mips64r6.s:12
zicond-primitiveSemantics_compare_reg_return_imm_reg-mips64r6.s:12
zicond-primitiveSemantics_compare_reg_return_reg_reg-mips64r6.s:6
zicond-primitiveSemantics_return_0_imm-mips64r6.s:6
zicond-primitiveSemantics_return_imm_imm-mips64r6.s:12
zicond-primitiveSemantics_return_imm_reg-mips64r6.s:6
zicond-primitiveSemantics_return_reg_reg-mips64r6.s:12
$
and e.g. (with the pseudo-op decorations removed):
primitiveSemantics_compare_imm_return_imm_imm_00:
xori $4,$4,0x2
li $3,4 # 0x4
li $2,7 # 0x7
jr $31
movn $2,$3,$4
or:
primitiveSemantics_compare_imm_return_imm_imm_00:
xori $4,$4,0x2
li $3,7 # 0x7
li $2,4 # 0x4
selnez $2,$2,$4
seleqz $4,$3,$4
jr $31
or $2,$2,$4
or:
primitiveSemantics_compare_imm_01:
xori $4,$4,0x2
move $2,$5
jr $31
movn $2,$0,$4
or:
primitiveSemantics_compare_imm_01:
xori $4,$4,0x2
jr $31
seleqz $2,$5,$4
Then with GCC 14:
$ grep -c movn zicond-*-mips4.s
zicond-conditionalArithmetic_compare_0_return_imm_reg-mips4.s:0
zicond-conditionalArithmetic_compare_0_return_reg_reg-mips4.s:0
zicond-conditionalArithmetic_compare_imm_return_imm_reg-mips4.s:0
zicond-conditionalArithmetic_compare_imm_return_reg_reg-mips4.s:0
zicond-conditionalArithmetic_compare_reg_return_imm_reg-mips4.s:0
zicond-conditionalArithmetic_compare_reg_return_reg_reg-mips4.s:0
zicond-primitiveSemantics-mips4.s:6
zicond-primitiveSemantics_compare_imm-mips4.s:0
zicond-primitiveSemantics_compare_imm_return_0_imm-mips4.s:0
zicond-primitiveSemantics_compare_imm_return_imm_imm-mips4.s:0
zicond-primitiveSemantics_compare_imm_return_imm_reg-mips4.s:0
zicond-primitiveSemantics_compare_imm_return_reg_reg-mips4.s:0
zicond-primitiveSemantics_compare_reg-mips4.s:0
zicond-primitiveSemantics_compare_reg_return_0_imm-mips4.s:0
zicond-primitiveSemantics_compare_reg_return_imm_imm-mips4.s:0
zicond-primitiveSemantics_compare_reg_return_imm_reg-mips4.s:0
zicond-primitiveSemantics_compare_reg_return_reg_reg-mips4.s:0
zicond-primitiveSemantics_return_0_imm-mips4.s:0
zicond-primitiveSemantics_return_imm_imm-mips4.s:0
zicond-primitiveSemantics_return_imm_reg-mips4.s:0
zicond-primitiveSemantics_return_reg_reg-mips4.s:6
$ grep -c seleqz zicond-*-mips64r6.s
zicond-conditionalArithmetic_compare_0_return_imm_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_0_return_reg_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_imm_return_imm_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_imm_return_reg_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_reg_return_imm_reg-mips64r6.s:0
zicond-conditionalArithmetic_compare_reg_return_reg_reg-mips64r6.s:0
zicond-primitiveSemantics-mips64r6.s:6
zicond-primitiveSemantics_compare_imm-mips64r6.s:6
zicond-primitiveSemantics_compare_imm_return_0_imm-mips64r6.s:6
zicond-primitiveSemantics_compare_imm_return_imm_imm-mips64r6.s:0
zicond-primitiveSemantics_compare_imm_return_imm_reg-mips64r6.s:0
zicond-primitiveSemantics_compare_imm_return_reg_reg-mips64r6.s:0
zicond-primitiveSemantics_compare_reg-mips64r6.s:6
zicond-primitiveSemantics_compare_reg_return_0_imm-mips64r6.s:6
zicond-primitiveSemantics_compare_reg_return_imm_imm-mips64r6.s:0
zicond-primitiveSemantics_compare_reg_return_imm_reg-mips64r6.s:0
zicond-primitiveSemantics_compare_reg_return_reg_reg-mips64r6.s:0
zicond-primitiveSemantics_return_0_imm-mips64r6.s:6
zicond-primitiveSemantics_return_imm_imm-mips64r6.s:0
zicond-primitiveSemantics_return_imm_reg-mips64r6.s:0
zicond-primitiveSemantics_return_reg_reg-mips64r6.s:0
$
and e.g. (again with the decorations removed):
primitiveSemantics_compare_imm_return_imm_imm_00:
li $2,2 # 0x2
beq $4,$2,.L6
nop
jr $31
li $2,4 # 0x4
.L6:
jr $31
li $2,7 # 0x7
or:
primitiveSemantics_compare_imm_return_imm_imm_00:
li $3,2 # 0x2
beqc $4,$3,.L7
li $2,4 # 0x4
jrc $31
.L7:
jr $31
li $2,7 # 0x7
or:
primitiveSemantics_compare_imm_01:
li $2,2 # 0x2
beq $4,$2,.L10
nop
jr $31
move $2,$0
.L10:
jr $31
move $2,$5
but:
primitiveSemantics_compare_imm_01:
xori $4,$4,0x2
jr $31
seleqz $2,$5,$4
So for the MIPS port the patch doesn't work, and regardless we have a bug
in the port affecting the output of MOVZ and SELNEZ, and finally we have a
functional regression from GCC 12 that prevents many of the conditional
moves from being produced too. Additionally the MIPS port continues to
require `-Wno-error' to build, also a regression.
However I have now withdrawn from MIPS/GCC development, so I will leave
the mess to the MIPS maintainer to sort out. I only made this analysis as
I find it useful for my RISC-V project anyway (the generic cond-move stuff
I mentioned at a previous patch review call).
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-21 13:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-02 17:20 [committed][RISC-V] Fix 20010221-1.c with zicond Jeff Law
2023-08-04 9:29 ` Xiao Zeng
2023-08-04 19:50 ` Jeff Law
2023-08-08 21:52 ` Maciej W. Rozycki
2023-08-08 22:45 ` Jeff Law
2023-08-21 13:33 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).