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
