RISC-V: Use clmulr for reversed crc64 on rv64
Commit Message
From: Loeka Rogge <loeka.rogge@gf.com>
This removes the guards that disables the clmulr pattern added by
patch r17-4206-ge91f79891fc657: Use clmulr for reversed crc32 on rv32
from being used on rv64.
This changes the calculation of the reflected quotient (on the host) to
fit inside 64 bits. gf2n_poly_long_div_quotient calculates
x^128 / (x^64 + lower order terms) = x^64 + lower order terms.
Meaning the result is missing an implicit 1 in front of the msb.
After reversing, this missing 1 should be after the lsb. This is
re-added with a shift and an or. This causes the msb to fall outside
the result stored in t0.
The value in t0 is only used by the emitted clmul or vclmul
instructions. Clmul only emits the bottom 64 bits, representing x^63 and
lower order terms. This is unaffected by the missing bit that
represents x^64, because it would always fall outside the result.
Regtested on rv32gc, rv32gc_zbc, rv64gc, and rv64gc_zbc.
gcc/ChangeLog:
* config/riscv/bitmanip.md: remove 64 bit guard.
* config/riscv/riscv.cc (riscv_optab_supported_p): remove
64 bit guard.
(expand_reversed_crc_using_clmul): Adapt quotient calculation
for 65 bits
* expr.cc (gf2n_poly_long_div_quotient): remove 64 bit guard
gcc/testsuite/ChangeLog:
* gcc.target/riscv/crc-builtin-zbc64.c: test whether clmulr is
applied for crc64
Signed-off-by: Loeka Rogge <loeka.rogge@globalfoundries.com>
---
gcc/config/riscv/bitmanip.md | 5 +--
gcc/config/riscv/riscv.cc | 36 ++++++++++++++-----
gcc/expr.cc | 6 ++--
.../gcc.target/riscv/crc-builtin-zbc64.c | 23 +++++++++++-
4 files changed, 54 insertions(+), 16 deletions(-)
Comments
On 10/2/26 3:20 AM, Michiel Derhaeg wrote:
> From: Loeka Rogge<loeka.rogge@gf.com>
>
> This removes the guards that disables the clmulr pattern added by
> patch r17-4206-ge91f79891fc657: Use clmulr for reversed crc32 on rv32
> from being used on rv64.
>
> This changes the calculation of the reflected quotient (on the host) to
> fit inside 64 bits. gf2n_poly_long_div_quotient calculates
> x^128 / (x^64 + lower order terms) = x^64 + lower order terms.
> Meaning the result is missing an implicit 1 in front of the msb.
>
> After reversing, this missing 1 should be after the lsb. This is
> re-added with a shift and an or. This causes the msb to fall outside
> the result stored in t0.
>
> The value in t0 is only used by the emitted clmul or vclmul
> instructions. Clmul only emits the bottom 64 bits, representing x^63 and
> lower order terms. This is unaffected by the missing bit that
> represents x^64, because it would always fall outside the result.
>
> Regtested on rv32gc, rv32gc_zbc, rv64gc, and rv64gc_zbc.
>
> gcc/ChangeLog:
>
> * config/riscv/bitmanip.md: remove 64 bit guard.
> * config/riscv/riscv.cc (riscv_optab_supported_p): remove
> 64 bit guard.
> (expand_reversed_crc_using_clmul): Adapt quotient calculation
> for 65 bits
> * expr.cc (gf2n_poly_long_div_quotient): remove 64 bit guard
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/crc-builtin-zbc64.c: test whether clmulr is
> applied for crc64
>
> Signed-off-by: Loeka Rogge<loeka.rogge@globalfoundries.com>
Thanks for following up on this. Just a few small things.
> ---
> gcc/config/riscv/bitmanip.md | 5 +--
> gcc/config/riscv/riscv.cc | 36 ++++++++++++++-----
> gcc/expr.cc | 6 ++--
> .../gcc.target/riscv/crc-builtin-zbc64.c | 23 +++++++++++-
> 4 files changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
> index 55bbf304bff..e3c00dd0e42 100644
> --- a/gcc/config/riscv/bitmanip.md
> +++ b/gcc/config/riscv/bitmanip.md
> @@ -1404,10 +1404,11 @@
> (E.g. CRC64's quotient may need 65 bits,
> we can't keep it in 64 bit variable.)
> then use clmul instruction to implement the CRC.
> - We can also use clmulr for CRC-32 on RV32, requiring ZBC. */
> + We can also use clmulr for CRC-32 on RV32 and CRC-64 on RV64,
> + requiring ZBC. */
Should the earlier part of the comment indicating we can't handle CRC64
due to the quotient potentially needing 64 bits need updating? THere's
a second copy a couple dozen lines later.
> if (((TARGET_ZBKC || TARGET_ZBC || TARGET_ZVBC)
> && <ANYI:MODE>mode < word_mode)
> - || (!TARGET_64BIT && TARGET_ZBC && <ANYI:MODE>mode == word_mode))
> + || (TARGET_ZBC && <ANYI:MODE>mode == word_mode))
> expand_reversed_crc_using_clmul (<ANYI:MODE>mode, <ANYI1:MODE>mode,
> operands);
> else
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index ae3fd8e3d72..1085a71ac75 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -13498,7 +13498,7 @@ riscv_optab_supported_p (int op, machine_mode, machine_mode result_mode,
> if (op == crc_rev_optab && opt_type != OPTIMIZE_FOR_SPEED)
> return (((TARGET_ZBKC || TARGET_ZBC || TARGET_ZVBC)
> && result_mode < word_mode)
> - || (!TARGET_64BIT && TARGET_ZBC && result_mode == word_mode));
> + || (TARGET_ZBC && result_mode == word_mode));
>
> return true;
> }
> @@ -15942,15 +15942,31 @@ expand_reversed_crc_using_clmul (scalar_mode crc_mode, scalar_mode data_mode,
> gcc_assert (!CONST_INT_P (operands[0]));
> gcc_assert (CONST_INT_P (operands[3]));
> unsigned short crc_size = GET_MODE_BITSIZE (crc_mode);
> - gcc_assert (crc_size <= 32);
> + gcc_assert (crc_size <= 64);
Would this be better checking crc_size <= BITS_PER_WORD? That would
guard against a 64bit crc on rv32. I don't think we can ever get into
this code in that scenario, but if we're going to have an assert, might
as well make it as tight as reasonably possible.
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index a9157dc5d41..0dd9d7d8870 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -14563,16 +14563,14 @@ int_expr_size (const_tree exp)
> }
>
> /* Return the quotient of polynomial long division of x^2N by POLYNOMIAL
> - in GF (2^N).
> + in GF (2^N). The result has degree N, so if N > 63, the result will
> + not fit in a HOST_WIDE_INT. This must be taken into account by the caller.
> Author: Richard Sandiford<richard.sandiford@arm.com> */
>
> unsigned HOST_WIDE_INT
> gf2n_poly_long_div_quotient (unsigned HOST_WIDE_INT polynomial,
> unsigned short n)
> {
> - /* The result has degree N, so needs N + 1 bits. */
> - gcc_assert (n < 64);
Would it make sense to test n <= 64 here?
LLM review suggested a CRC64 test run test with a dense polynomial
like 0x42f0e1eba9ea3693ull as well as initial values that have some
bits set in 32..63 range. Not strictly required, but nice to have.
No concerns about the core of the patch. Inclined to ack with the
assert adjustments and comment fixes. Love to have a 64bit run test,
but I won't demand it.
jeff
Hi Jeff,
Jeff Law <jeffrey.law@oss.qualcomm.com> writes:
> On 10/2/26 3:20 AM, Michiel Derhaeg wrote:
>> diff --git a/gcc/expr.cc b/gcc/expr.cc
>> index a9157dc5d41..0dd9d7d8870 100644
>> --- a/gcc/expr.cc
>> +++ b/gcc/expr.cc
>> @@ -14563,16 +14563,14 @@ int_expr_size (const_tree exp)
>> }
>>
>> /* Return the quotient of polynomial long division of x^2N by POLYNOMIAL
>> - in GF (2^N).
>> + in GF (2^N). The result has degree N, so if N > 63, the result will
>> + not fit in a HOST_WIDE_INT. This must be taken into account by the caller.
>> Author: Richard Sandiford<richard.sandiford@arm.com> */
>>
>> unsigned HOST_WIDE_INT
>> gf2n_poly_long_div_quotient (unsigned HOST_WIDE_INT polynomial,
>> unsigned short n)
>> {
>> - /* The result has degree N, so needs N + 1 bits. */
>> - gcc_assert (n < 64);
> Would it make sense to test n <= 64 here?
That's what Loeka's first version did. But I felt that it was a bit
arbitrary. For every degree higher than 63, 1 bit of information is lost
in the result. So why draw the line at 1?
Thanks!
Michiel
On 10/10/26 10:08 AM, Michiel Derhaeg wrote:
> Hi Jeff,
>
> Jeff Law <jeffrey.law@oss.qualcomm.com> writes:
>
>> On 10/2/26 3:20 AM, Michiel Derhaeg wrote:
>>> diff --git a/gcc/expr.cc b/gcc/expr.cc
>>> index a9157dc5d41..0dd9d7d8870 100644
>>> --- a/gcc/expr.cc
>>> +++ b/gcc/expr.cc
>>> @@ -14563,16 +14563,14 @@ int_expr_size (const_tree exp)
>>> }
>>>
>>> /* Return the quotient of polynomial long division of x^2N by POLYNOMIAL
>>> - in GF (2^N).
>>> + in GF (2^N). The result has degree N, so if N > 63, the result will
>>> + not fit in a HOST_WIDE_INT. This must be taken into account by the caller.
>>> Author: Richard Sandiford<richard.sandiford@arm.com> */
>>>
>>> unsigned HOST_WIDE_INT
>>> gf2n_poly_long_div_quotient (unsigned HOST_WIDE_INT polynomial,
>>> unsigned short n)
>>> {
>>> - /* The result has degree N, so needs N + 1 bits. */
>>> - gcc_assert (n < 64);
>> Would it make sense to test n <= 64 here?
> That's what Loeka's first version did. But I felt that it was a bit
> arbitrary. For every degree higher than 63, 1 bit of information is lost
> in the result. So why draw the line at 1?
But does it work for those higher degrees? If it does, then the
assertion should be removed, if not, then we probably want to keep the
adjusted assert.
Jeff
Jeff Law <jeffrey.law@oss.qualcomm.com> writes:
> On 10/10/26 10:08 AM, Michiel Derhaeg wrote:
>> Hi Jeff,
>>
>> Jeff Law <jeffrey.law@oss.qualcomm.com> writes:
>>
>>> On 10/2/26 3:20 AM, Michiel Derhaeg wrote:
>>>> diff --git a/gcc/expr.cc b/gcc/expr.cc
>>>> index a9157dc5d41..0dd9d7d8870 100644
>>>> --- a/gcc/expr.cc
>>>> +++ b/gcc/expr.cc
>>>> @@ -14563,16 +14563,14 @@ int_expr_size (const_tree exp)
>>>> }
>>>>
>>>> /* Return the quotient of polynomial long division of x^2N by POLYNOMIAL
>>>> - in GF (2^N).
>>>> + in GF (2^N). The result has degree N, so if N > 63, the result will
>>>> + not fit in a HOST_WIDE_INT. This must be taken into account by the caller.
>>>> Author: Richard Sandiford<richard.sandiford@arm.com> */
>>>>
>>>> unsigned HOST_WIDE_INT
>>>> gf2n_poly_long_div_quotient (unsigned HOST_WIDE_INT polynomial,
>>>> unsigned short n)
>>>> {
>>>> - /* The result has degree N, so needs N + 1 bits. */
>>>> - gcc_assert (n < 64);
>>> Would it make sense to test n <= 64 here?
>> That's what Loeka's first version did. But I felt that it was a bit
>> arbitrary. For every degree higher than 63, 1 bit of information is lost
>> in the result. So why draw the line at 1?
> But does it work for those higher degrees?
For some reason I believed it did, but that turns out to be nonsense.
We'll reintroduce the assert.
Thanks,
Michiel
Resending. Original didn't seem to hit the mailing list...
Jeff Law <jeffrey.law@oss.qualcomm.com> writes:
> On 10/10/26 10:08 AM, Michiel Derhaeg wrote:
>> Hi Jeff,
>>
>> Jeff Law <jeffrey.law@oss.qualcomm.com> writes:
>>
>>> On 10/2/26 3:20 AM, Michiel Derhaeg wrote:
>>>> diff --git a/gcc/expr.cc b/gcc/expr.cc
>>>> index a9157dc5d41..0dd9d7d8870 100644
>>>> --- a/gcc/expr.cc
>>>> +++ b/gcc/expr.cc
>>>> @@ -14563,16 +14563,14 @@ int_expr_size (const_tree exp)
>>>> }
>>>>
>>>> /* Return the quotient of polynomial long division of x^2N by POLYNOMIAL
>>>> - in GF (2^N).
>>>> + in GF (2^N). The result has degree N, so if N > 63, the result will
>>>> + not fit in a HOST_WIDE_INT. This must be taken into account by the caller.
>>>> Author: Richard Sandiford<richard.sandiford@arm.com> */
>>>>
>>>> unsigned HOST_WIDE_INT
>>>> gf2n_poly_long_div_quotient (unsigned HOST_WIDE_INT polynomial,
>>>> unsigned short n)
>>>> {
>>>> - /* The result has degree N, so needs N + 1 bits. */
>>>> - gcc_assert (n < 64);
>>> Would it make sense to test n <= 64 here?
>> That's what Loeka's first version did. But I felt that it was a bit
>> arbitrary. For every degree higher than 63, 1 bit of information is lost
>> in the result. So why draw the line at 1?
> But does it work for those higher degrees?
For some reason I believed it did, but that seems to be nonsense.
We'll reintroduce the assert.
Thanks,
Michiel
@@ -1404,10 +1404,11 @@
(E.g. CRC64's quotient may need 65 bits,
we can't keep it in 64 bit variable.)
then use clmul instruction to implement the CRC.
- We can also use clmulr for CRC-32 on RV32, requiring ZBC. */
+ We can also use clmulr for CRC-32 on RV32 and CRC-64 on RV64,
+ requiring ZBC. */
if (((TARGET_ZBKC || TARGET_ZBC || TARGET_ZVBC)
&& <ANYI:MODE>mode < word_mode)
- || (!TARGET_64BIT && TARGET_ZBC && <ANYI:MODE>mode == word_mode))
+ || (TARGET_ZBC && <ANYI:MODE>mode == word_mode))
expand_reversed_crc_using_clmul (<ANYI:MODE>mode, <ANYI1:MODE>mode,
operands);
else
@@ -13498,7 +13498,7 @@ riscv_optab_supported_p (int op, machine_mode, machine_mode result_mode,
if (op == crc_rev_optab && opt_type != OPTIMIZE_FOR_SPEED)
return (((TARGET_ZBKC || TARGET_ZBC || TARGET_ZVBC)
&& result_mode < word_mode)
- || (!TARGET_64BIT && TARGET_ZBC && result_mode == word_mode));
+ || (TARGET_ZBC && result_mode == word_mode));
return true;
}
@@ -15942,15 +15942,31 @@ expand_reversed_crc_using_clmul (scalar_mode crc_mode, scalar_mode data_mode,
gcc_assert (!CONST_INT_P (operands[0]));
gcc_assert (CONST_INT_P (operands[3]));
unsigned short crc_size = GET_MODE_BITSIZE (crc_mode);
- gcc_assert (crc_size <= 32);
+ gcc_assert (crc_size <= 64);
unsigned short data_size = GET_MODE_BITSIZE (data_mode);
rtx polynomial = operands[3];
- /* Calculate the quotient. */
- unsigned HOST_WIDE_INT
- q = gf2n_poly_long_div_quotient (UINTVAL (polynomial), crc_size);
- /* Reflect the calculated quotient. */
- q = reflect_hwi (q, crc_size + 1);
+ unsigned HOST_WIDE_INT q;
+ if (crc_size == 64)
+ {
+ /* Calculate the quotient.
+ x^64 is left implicit at the msb to fit in 64 bits. */
+ q = gf2n_poly_long_div_quotient (UINTVAL (polynomial), crc_size);
+ /* Reflect the calculated quotient.
+ Implicit bit is now missing at the lsb. */
+ q = reflect_hwi (q, crc_size);
+ /* Add the implicit bit back at the lsb.
+ This is okay because the extra bit falls outside the 64 lsbs output
+ by clmul. */
+ q = (q << 1) | 1;
+ }
+ else
+ {
+ /* Calculate the quotient. */
+ q = gf2n_poly_long_div_quotient (UINTVAL (polynomial), crc_size);
+ /* Reflect the calculated quotient. */
+ q = reflect_hwi (q, crc_size + 1);
+ }
rtx t0 = gen_reg_rtx (word_mode);
riscv_emit_move (t0, gen_int_mode (q, word_mode));
@@ -15989,8 +16005,10 @@ expand_reversed_crc_using_clmul (scalar_mode crc_mode, scalar_mode data_mode,
if (use_clmulr)
{
- gcc_assert (!TARGET_64BIT);
- emit_insn (gen_riscv_clmulr_si (a0, a0, t1));
+ if (TARGET_64BIT)
+ emit_insn (gen_riscv_clmulr_di (a0, a0, t1));
+ else
+ emit_insn (gen_riscv_clmulr_si (a0, a0, t1));
}
else
{
@@ -14563,16 +14563,14 @@ int_expr_size (const_tree exp)
}
/* Return the quotient of polynomial long division of x^2N by POLYNOMIAL
- in GF (2^N).
+ in GF (2^N). The result has degree N, so if N > 63, the result will
+ not fit in a HOST_WIDE_INT. This must be taken into account by the caller.
Author: Richard Sandiford <richard.sandiford@arm.com> */
unsigned HOST_WIDE_INT
gf2n_poly_long_div_quotient (unsigned HOST_WIDE_INT polynomial,
unsigned short n)
{
- /* The result has degree N, so needs N + 1 bits. */
- gcc_assert (n < 64);
-
/* Perform a division step for the x^2N coefficient. At this point the
quotient and remainder have N implicit trailing zeros. */
unsigned HOST_WIDE_INT quotient = 1;
@@ -63,5 +63,26 @@ int32_t rev_crc32_data32 (int32_t x)
{
return __builtin_rev_crc32_data32 (x, 0x123546ff, 0x4002123);
}
-/* { dg-final { scan-assembler-times "clmul\t" 18 } } */
+
+int64_t rev_crc64_data8 (int64_t x)
+{
+ return __builtin_rev_crc64_data8 (x, 0x32, 0x4002123);
+}
+
+int64_t rev_crc64_data16 (int64_t x)
+{
+ return __builtin_rev_crc64_data16 (x, 0x3232, 0x4002123);
+}
+
+int64_t rev_crc64_data32 (int64_t x)
+{
+ return __builtin_rev_crc64_data32 (x, 0x123546ff, 0x4002123);
+}
+
+int64_t rev_crc64_data64 (int64_t x)
+{
+ return __builtin_rev_crc64_data64 (x, 0x123546ff123546ff, 0x4002123);
+}
+/* { dg-final { scan-assembler-times "clmul\t" 22 } } */
/* { dg-final { scan-assembler-times "clmulh" 6 } } */
+/* { dg-final { scan-assembler-times "clmulr" 4 } } */