On Sun, Nov 13, 2022 at 10:46:34PM +0100, Christoph Muellner wrote:
> +(define_expand "extv<mode>"
> + [(set (match_operand:GPR 0 "register_operand" "=r")
> + (sign_extract:GPR (match_operand:GPR 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand")
> + (match_operand 3 "const_int_operand")))]
> + "TARGET_XTHEADBB"
> +{
> + if (TARGET_XTHEADBB
> + && ((INTVAL (operands[2]) + INTVAL (operands[3]))
> + >= GET_MODE_BITSIZE (GET_MODE (operands[1])).to_constant ()))
> + FAIL;
> +})
> +
> +(define_expand "extzv<mode>"
> + [(set (match_operand:GPR 0 "register_operand" "=r")
> + (zero_extract:GPR (match_operand:GPR 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand")
> + (match_operand 3 "const_int_operand")))]
> + "TARGET_XTHEADBB"
> +{
> + if (TARGET_XTHEADBB
> + && ((INTVAL (operands[2]) + INTVAL (operands[3]))
> + >= GET_MODE_BITSIZE (GET_MODE (operands[1])).to_constant ()))
> + FAIL;
I doubt whether it is necessary to add this judgment here,
and other architectures seem to have not added it. But there's nothing wrong
with adding
> +
> +(define_insn "*th_ext<mode>"
> + [(set (match_operand:X 0 "register_operand" "=r")
> + (sign_extract:X (match_operand:X 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand")
> + (match_operand 3 "const_int_operand")))]
> + "TARGET_XTHEADBB"
> +{
> + operands[3] = GEN_INT (INTVAL (operands[2]) + INTVAL (operands[3]));
> + return "th.ext\t%0,%1,%2,%3";
> +}
> + [(set_attr "type" "bitmanip")])
> +
> +(define_insn "*th_extu<mode>"
> + [(set (match_operand:X 0 "register_operand" "=r")
> + (zero_extract:X (match_operand:X 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand")
> + (match_operand 3 "const_int_operand")))]
> + "TARGET_XTHEADBB"
> +{
> + operands[3] = GEN_INT (INTVAL (operands[2]) + INTVAL (operands[3]));
> + return "th.extu\t%0,%1,%2,%3";
> +}
> + [(set_attr "type" "bitmanip")])
> +
I think the operands[3] should be:
operands[3] = GEN_INT (INTVAL (operands[2]) + INTVAL (operands[3])) - 1
Because the ext and extu extract the bits %2..%3, when size is 1, the 2%
equals to 3%.
And a small optimization can be done here, the extzv can generate c.andi
when the start bit is 0 and the size is less than 7.
> +/* { dg-final { scan-assembler-times "th.revw\t" 2 } } */
> +/* { dg-final { scan-assembler-times "th.rev\t" 2 } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-srri.c
> b/gcc/testsuite/gcc.target/riscv/xtheadbb-srri.c
> new file mode 100644
> index 00000000000..cd992ae3f0a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-srri.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xtheadbb -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-g" } } */
> +
> +unsigned long foo1(unsigned long rs1)
> +{
> + long shamt = __riscv_xlen - 11;
> + return (rs1 << shamt) |
> + (rs1 >> ((__riscv_xlen - shamt) & (__riscv_xlen - 1)));
> +}
> +unsigned long foo2(unsigned long rs1)
> +{
> + unsigned long shamt = __riscv_xlen - 11;
> + return (rs1 >> shamt) |
> + (rs1 << ((__riscv_xlen - shamt) & (__riscv_xlen - 1)));
> +}
> +
> +/* { dg-final { scan-assembler-times "th.srri" 2 } } */
Why is there no testcase for ff1 here? It can be generated by the builtin
function '__builtin_clzl'.
Thanks,
Cooper
