Re: [PATCHv3 2/2] aarch64: Add support for _BitInt
"Andre Vieira (lists)" writes:
> Added the target check, also had to change some of the assembly checking
> due to changes upstream, the assembly is still valid, but we do extend
> where not necessary, I do believe that's a general issue though.
>
> The _BitInt(N > 64) codegen for non-powers of 2 did get worse, we see
> similar codegen with _int128 bitfields on aarch64.
> I suspect we need to improve the way we 'extend' TImode in the aarch64
> backend to be able to operate only on the affected DImode parts of it
> when relevant. Though I also think we may need to change how _BitInt is
> currently expanded in such situations, right now it does the extension
> as two shifts. Anyway I did not have too much time to look deeper into this.
>
> Bootstrapped on aarch64-unknown-linux-gnu.
>
> OK for trunk?
OK, thanks. In truth I've not gone through the tests very thorougly
this time around, and just gone by the internal diff between this
version and the previous one. But we can adjust them as necessary
based on any reports that come in.
Richard
>
> On 28/03/2024 15:21, Richard Sandiford wrote:
>> Jakub Jelinek writes:
>>> On Thu, Mar 28, 2024 at 03:00:46PM +, Richard Sandiford wrote:
> * gcc.target/aarch64/bitint-alignments.c: New test.
> * gcc.target/aarch64/bitint-args.c: New test.
> * gcc.target/aarch64/bitint-sizes.c: New test.
> * gcc.target/aarch64/bitfield-bitint-abi.h: New header.
> * gcc.target/aarch64/bitfield-bitint-abi-align16.c: New test.
> * gcc.target/aarch64/bitfield-bitint-abi-align8.c: New test.
Since we don't support big-endian yet, I assume the tests should be
conditional on aarch64_little_endian.
>>>
>>> Perhaps better on bitint effective target, then they'll become available
>>> automatically as soon as big endian aarch64 _BitInt support is turned on.
>>
>> Ah, yeah, good point.
>>
>> Richard
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index
> 81400cc666472ffeff40df14e98ae00ebc774d31..c0af4ef151a8c46f78c0c3a43c2ab1318a3f610a
> 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -6583,6 +6583,7 @@ aarch64_return_in_memory_1 (const_tree type)
>int count;
>
>if (!AGGREGATE_TYPE_P (type)
> + && TREE_CODE (type) != BITINT_TYPE
>&& TREE_CODE (type) != COMPLEX_TYPE
>&& TREE_CODE (type) != VECTOR_TYPE)
> /* Simple scalar types always returned in registers. */
> @@ -21996,6 +21997,11 @@ aarch64_composite_type_p (const_tree type,
>if (type && (AGGREGATE_TYPE_P (type) || TREE_CODE (type) == COMPLEX_TYPE))
> return true;
>
> + if (type
> + && TREE_CODE (type) == BITINT_TYPE
> + && int_size_in_bytes (type) > 16)
> +return true;
> +
>if (mode == BLKmode
>|| GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT
>|| GET_MODE_CLASS (mode) == MODE_COMPLEX_INT)
> @@ -28477,6 +28483,42 @@ aarch64_excess_precision (enum excess_precision_type
> type)
>return FLT_EVAL_METHOD_UNPREDICTABLE;
> }
>
> +/* Implement TARGET_C_BITINT_TYPE_INFO.
> + Return true if _BitInt(N) is supported and fill its details into *INFO.
> */
> +bool
> +aarch64_bitint_type_info (int n, struct bitint_info *info)
> +{
> + if (TARGET_BIG_END)
> +return false;
> +
> + if (n <= 8)
> +info->limb_mode = QImode;
> + else if (n <= 16)
> +info->limb_mode = HImode;
> + else if (n <= 32)
> +info->limb_mode = SImode;
> + else if (n <= 64)
> +info->limb_mode = DImode;
> + else if (n <= 128)
> +info->limb_mode = TImode;
> + else
> +/* The AAPCS for AArch64 defines _BitInt(N > 128) as an array with
> + type {signed,unsigned} __int128[M] where M*128 >= N. However, to be
> + able to use libgcc's implementation to support large _BitInt's we need
> + to use a LIMB_MODE that is no larger than 'long long'. This is why we
> + use DImode for our internal LIMB_MODE and we define the ABI_LIMB_MODE
> to
> + be TImode to ensure we are ABI compliant. */
> +info->limb_mode = DImode;
> +
> + if (n > 128)
> +info->abi_limb_mode = TImode;
> + else
> +info->abi_limb_mode = info->limb_mode;
> + info->big_endian = TARGET_BIG_END;
> + info->extended = false;
> + return true;
> +}
> +
> /* Implement TARGET_SCHED_CAN_SPECULATE_INSN. Return true if INSN can be
> scheduled for speculative execution. Reject the long-running division
> and square-root instructions. */
> @@ -30601,6 +30643,9 @@ aarch64_run_selftests (void)
> #undef TARGET_C_EXCESS_PRECISION
> #define TARGET_C_EXCESS_PRECISION aarch64_excess_precision
>
> +#undef TARGET_C_BITINT_TYPE_INFO
> +#define TARGET_C_BITINT_TYPE_INFO aarch64_bitint_type_info
> +
> #undef TARGET_EXPAND_BUILTIN
> #define TARGET_EXPAND_BUILTIN aarch64_expand_builtin
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-bitint-abi-align16.c
> b/gcc/testsuite/gcc.target/aarch64/bitfield-bitint-abi-align16.c
>
Re: [PATCHv3 2/2] aarch64: Add support for _BitInt
Added the target check, also had to change some of the assembly checking
due to changes upstream, the assembly is still valid, but we do extend
where not necessary, I do believe that's a general issue though.
The _BitInt(N > 64) codegen for non-powers of 2 did get worse, we see
similar codegen with _int128 bitfields on aarch64.
I suspect we need to improve the way we 'extend' TImode in the aarch64
backend to be able to operate only on the affected DImode parts of it
when relevant. Though I also think we may need to change how _BitInt is
currently expanded in such situations, right now it does the extension
as two shifts. Anyway I did not have too much time to look deeper into this.
Bootstrapped on aarch64-unknown-linux-gnu.
OK for trunk?
On 28/03/2024 15:21, Richard Sandiford wrote:
Jakub Jelinek writes:
On Thu, Mar 28, 2024 at 03:00:46PM +, Richard Sandiford wrote:
* gcc.target/aarch64/bitint-alignments.c: New test.
* gcc.target/aarch64/bitint-args.c: New test.
* gcc.target/aarch64/bitint-sizes.c: New test.
* gcc.target/aarch64/bitfield-bitint-abi.h: New header.
* gcc.target/aarch64/bitfield-bitint-abi-align16.c: New test.
* gcc.target/aarch64/bitfield-bitint-abi-align8.c: New test.
Since we don't support big-endian yet, I assume the tests should be
conditional on aarch64_little_endian.
Perhaps better on bitint effective target, then they'll become available
automatically as soon as big endian aarch64 _BitInt support is turned on.
Ah, yeah, good point.
Richarddiff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index
81400cc666472ffeff40df14e98ae00ebc774d31..c0af4ef151a8c46f78c0c3a43c2ab1318a3f610a
100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -6583,6 +6583,7 @@ aarch64_return_in_memory_1 (const_tree type)
int count;
if (!AGGREGATE_TYPE_P (type)
+ && TREE_CODE (type) != BITINT_TYPE
&& TREE_CODE (type) != COMPLEX_TYPE
&& TREE_CODE (type) != VECTOR_TYPE)
/* Simple scalar types always returned in registers. */
@@ -21996,6 +21997,11 @@ aarch64_composite_type_p (const_tree type,
if (type && (AGGREGATE_TYPE_P (type) || TREE_CODE (type) == COMPLEX_TYPE))
return true;
+ if (type
+ && TREE_CODE (type) == BITINT_TYPE
+ && int_size_in_bytes (type) > 16)
+return true;
+
if (mode == BLKmode
|| GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT
|| GET_MODE_CLASS (mode) == MODE_COMPLEX_INT)
@@ -28477,6 +28483,42 @@ aarch64_excess_precision (enum excess_precision_type
type)
return FLT_EVAL_METHOD_UNPREDICTABLE;
}
+/* Implement TARGET_C_BITINT_TYPE_INFO.
+ Return true if _BitInt(N) is supported and fill its details into *INFO. */
+bool
+aarch64_bitint_type_info (int n, struct bitint_info *info)
+{
+ if (TARGET_BIG_END)
+return false;
+
+ if (n <= 8)
+info->limb_mode = QImode;
+ else if (n <= 16)
+info->limb_mode = HImode;
+ else if (n <= 32)
+info->limb_mode = SImode;
+ else if (n <= 64)
+info->limb_mode = DImode;
+ else if (n <= 128)
+info->limb_mode = TImode;
+ else
+/* The AAPCS for AArch64 defines _BitInt(N > 128) as an array with
+ type {signed,unsigned} __int128[M] where M*128 >= N. However, to be
+ able to use libgcc's implementation to support large _BitInt's we need
+ to use a LIMB_MODE that is no larger than 'long long'. This is why we
+ use DImode for our internal LIMB_MODE and we define the ABI_LIMB_MODE to
+ be TImode to ensure we are ABI compliant. */
+info->limb_mode = DImode;
+
+ if (n > 128)
+info->abi_limb_mode = TImode;
+ else
+info->abi_limb_mode = info->limb_mode;
+ info->big_endian = TARGET_BIG_END;
+ info->extended = false;
+ return true;
+}
+
/* Implement TARGET_SCHED_CAN_SPECULATE_INSN. Return true if INSN can be
scheduled for speculative execution. Reject the long-running division
and square-root instructions. */
@@ -30601,6 +30643,9 @@ aarch64_run_selftests (void)
#undef TARGET_C_EXCESS_PRECISION
#define TARGET_C_EXCESS_PRECISION aarch64_excess_precision
+#undef TARGET_C_BITINT_TYPE_INFO
+#define TARGET_C_BITINT_TYPE_INFO aarch64_bitint_type_info
+
#undef TARGET_EXPAND_BUILTIN
#define TARGET_EXPAND_BUILTIN aarch64_expand_builtin
diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-bitint-abi-align16.c
b/gcc/testsuite/gcc.target/aarch64/bitfield-bitint-abi-align16.c
new file mode 100644
index
..3f292a45f955d35b802a0bd789cd39d5fa7b5860
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/bitfield-bitint-abi-align16.c
@@ -0,0 +1,384 @@
+/* { dg-do compile { target bitint } } */
+/* { dg-additional-options "-std=c23 -O2 -fno-stack-protector -save-temps
-fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+#define ALIGN 16
+#include "bitfield-bitint-abi.h"
+
+// f1-f16 are all the same
+
