<saurabh....@arm.com> writes:
This patch introduces support for LUTI2/LUTI4 ACLE for SVE2.
LUTI instructions are used for efficient table lookups with 2-bit
or 4-bit indices. LUTI2 reads indexed 8-bit or 16-bit elements from
the low 128 bits of the table vector using packed 2-bit indices,
while LUTI4 can read from the low 128 or 256 bits of the table
vector or from two table vectors using packed 4-bit indices.
These instructions fill the destination vector by copying elements
indexed by segments of the source vector, selected by the vector
segment index.
The changes include the addition of a new AArch64 option
extension "lut", __ARM_FEATURE_LUT preprocessor macro, definitions
for the new LUTI instruction shapes, and implementations of the
svluti2 and svluti4 builtins.
New tests are added as well.
---
Hey,
This is a respin of
https://gcc.gnu.org/pipermail/gcc-patches/2024-July/658015.html. Rebased
with master. Regression tested on aarch64-unknown-linux-gnu and found no
regressions.
Thanks for picking this up! The issues described below are of course not
your fault :)
Ok for master?
Thanks,
Saurabh
---
[...]
diff --git a/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
b/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
index ca721dd2c09..0f6d366b2d6 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins-shapes.cc
@@ -903,6 +903,47 @@ struct load_ext_gather_base : public overloaded_base<1>
}
};
+
+/* sv<v0>_t svlut_<t0>(sv<t0>_t, svuint8_t, uint64_t)
Because of the potential tuple argument, I suppose this should be:
sv<v0>_t svlut[_<t0>_g](sv<t0>x<g>_t, svuint8_t, uint64_t)
Unlike for the ZT version of svluti, the type suffix is optional.
To quote from the ACLE spec:
svint8_t svluti2_lane[_s8](svint8_t table, svuint8_t indices, uint64_t
imm_idx);
svint16_t svluti2_lane[_s16]( svint16_t table, svuint8_t indices, uint64_t
imm_idx);
svint8_t svluti4_lane[_s8](svint8_t table, svuint8_t indices, uint64_t
imm_idx);
svint16_t svluti4_lane[_s16](svint16_t table, svuint8_t indices, uint64_t
imm_idx);
svint16_t svluti4_lane[_s16_x2](svint16x2_t table, svuint8_t indices,
uint64_t imm_idx);
Because of that:
+ where the final argument is a constant index, the instruction divides
+ the vector argument in BITS-bit quantities. */
+template<unsigned int BITS>
+struct luti_base : public nonoverloaded_base
...this should be an overloaded function and have a resolver.
+{
+ void
+ build (function_builder &b, const function_group_info &group) const override
+ {
+ /* Format: return type, table vector, indices vector, immediate value. */
+ build_all (b, "v0,t0,vu8,su64", group, MODE_none);
+ }
+
+ bool
+ check (function_checker &c) const override
+ {
+ int max_range;
+ bool byte_mode = c.type_suffix (0).element_bits == 8;
+
+ if (BITS == 2)
+ max_range = byte_mode ? 3 : 7;
+ else if (BITS == 4)
+ max_range = byte_mode ? 1 : 7;
It looks like this should be ? 1 : 3, see:
https://developer.arm.com/documentation/ddi0602/2024-12/SVE-Instructions/LUTI4--Lookup-table-read-with-4-bit-indices-?lang=en
Or, programmatically, I think this is:
auto max_range = c.type.suffix (0).element_bits / BITS - 1;
for all cases.
+ else
+ /* Unsupported number of indices bits for LUTI. */
+ gcc_unreachable ();
+
+ return c.require_immediate_range (2, 0, max_range);
+ }
+
+};
+
+/* Specializations for 2-bit and 4-bit indices. */
+using luti2_def = luti_base<2>;
+SHAPE (luti2)
+
+using luti4_def = luti_base<4>;
+SHAPE (luti4)
+
+
/* sv<t0>x<g>_t svfoo_t0_g(uint64_t, svuint8_t, uint64_t)
where the first argument is the ZT register number (currently always 0)
[...]
diff --git a/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
b/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
index e726fa1fb68..0c4f8251ac0 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
+++ b/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
@@ -164,6 +164,10 @@ DEF_SVE_FUNCTION (svwhilegt, compare_scalar, while, none)
DEF_SVE_FUNCTION (svwhilerw, compare_ptr, all_data, none)
DEF_SVE_FUNCTION (svwhilewr, compare_ptr, all_data, none)
DEF_SVE_FUNCTION (svxar, ternary_shift_right_imm, all_integer, none)
+DEF_SVE_FUNCTION (svluti2_lane, luti2, bhs_data, none)
+DEF_SVE_FUNCTION (svluti4_lane, luti4, bhs_data, none)
+DEF_SVE_FUNCTION_GS (svluti4_lane, luti4, bhs_data, x2, none)
bhs_data looks wrong: there should be no .s versions. Similarly...
+
#undef REQUIRED_EXTENSIONS
#define REQUIRED_EXTENSIONS nonstreaming_sve (AARCH64_FL_SVE2)
[...]
diff --git a/gcc/config/aarch64/aarch64-sve2.md
b/gcc/config/aarch64/aarch64-sve2.md
index f8cfe08f4c0..7dcbc0700da 100644
--- a/gcc/config/aarch64/aarch64-sve2.md
+++ b/gcc/config/aarch64/aarch64-sve2.md
@@ -133,6 +133,7 @@
;; ---- Optional AES extensions
;; ---- Optional SHA-3 extensions
;; ---- Optional SM4 extensions
+;; ---- Table lookup
;; =========================================================================
;; == Moves
@@ -4211,3 +4212,47 @@
"sm4ekey\t%0.s, %1.s, %2.s"
[(set_attr "type" "crypto_sm4")]
)
+
+;; -------------------------------------------------------------------------
+;; ---- Table lookup
+;; -------------------------------------------------------------------------
+;; Includes:
+;; - LUTI2
+;; - LUTI4
+;; -------------------------------------------------------------------------
+
+(define_insn "@aarch64_sve_luti<LUTI_BITS><mode>"
+ [(set (match_operand:SVE_FULL_BS 0 "register_operand" "=w")
+ (unspec:SVE_FULL_BS
+ [(match_operand:SVE_FULL_BS 1 "register_operand" "w")
+ (match_operand:VNx16QI 2 "register_operand" "w")
+ (match_operand:DI 3 "const_int_operand")
+ (const_int LUTI_BITS)]
+ UNSPEC_SVE_LUTI))]
+ "TARGET_SVE2"
+ "luti<LUTI_BITS>\t%0.<Vetype>, { %1.<Vetype> }, %2[%3]"
+)
+
+(define_insn "@aarch64_sve_luti<LUTI_BITS><mode>"
+ [(set (match_operand:<VSINGLE> 0 "register_operand" "=w")
+ (unspec:<VSINGLE>
+ [(match_operand:SVE_FULL_H 1 "aligned_register_operand" "w")
+ (match_operand:VNx16QI 2 "register_operand" "w")
+ (match_operand:DI 3 "const_int_operand")
+ (const_int LUTI_BITS)]
+ UNSPEC_SVE_LUTI))]
+ "TARGET_SVE2"
+ "luti<LUTI_BITS>\t%0.<Vetype>, { %1.<Vetype> }, %2[%3]"
+)
...there should be .S (VNx4) variants here. Also, the .H variants
don't require an aligned register operand. (FWIW, using "w" with
"aligned_register_operand" is wrong in any case, since "w" accepts
unaligned registers.)
It looks like we could merge the patterns above into a single SVE_FULL_BH
pattern.
+
+(define_insn "@aarch64_sve_luti<LUTI_BITS><mode>"
+ [(set (match_operand:<VSINGLE> 0 "register_operand" "=w")
+ (unspec:<VSINGLE>
+ [(match_operand:SVE_FULL_Hx2 1 "aligned_register_operand" "Uw2")
This operand also isn't required to be aligned: Zn has a 5-bit encoding.
+ (match_operand:VNx16QI 2 "register_operand" "w")
+ (match_operand:DI 3 "const_int_operand")
+ (const_int LUTI_BITS)]
+ UNSPEC_SVE_LUTI))]
+ "TARGET_SVE2"
+ "luti<LUTI_BITS>\t%0.<Vetype>, %1, %2[%3]"
+)
Also, formatting nit, but: it's more usual to indent the "[" in an
unspec by 2 or 1 extra columns, rather than a full tab.
[...]
diff --git a/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/luti2_bf16.c
b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/luti2_bf16.c
new file mode 100644
index 00000000000..f423bfae2c6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve2/acle/asm/luti2_bf16.c
@@ -0,0 +1,40 @@
+/* { dg-do compile } */
This would be better as:
/* { dg-do assemble { target aarch64_asm_lut_ok } } */
/* { dg-do compile { target { ! aarch64_asm_lut_ok } } } */
with lut added to:
foreach { aarch64_ext } { "fp" "simd" "crypto" "crc" "lse" "dotprod" "sve"
"i8mm" "f32mm" "f64mm" "bf16" "sb" "sve2" "ls64"
"sme" "sme-i16i64" "sme2" "sve-b16b16"
"sme-b16b16" "sme-f16f16" "sme2p1" "fp8" "fp8fma"
"ssve-fp8fma" "fp8dot2" "ssve-fp8dot2" "fp8dot4"
"ssve-fp8dot4"} {
eval [string map [list FUNC $aarch64_ext] {
proc check_effective_target_aarch64_asm_FUNC_ok { } {
if { [istarget aarch64*-*-*] } {
return [check_no_compiler_messages aarch64_FUNC_assembler
object {
__asm__ (".arch_extension FUNC");
} "-march=armv8-a+FUNC"]
} else {
return 0
}
}
}]
}
in target-supports.exp.
+/* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
+
+#include "test_sve_acle.h"
+
+#pragma GCC target "+sve2+lut"
+#if STREAMING_COMPATIBLE
+#pragma GCC target "+sme2"
+#endif
+
+/*
+** luti2_test_imm0:
+** luti2 z1\.h, \{ z28\.h \}, z0\[0\]
+** ret
+*/
+
+TEST_XN_SINGLE (luti2_test_imm0, svbfloat16_t, svuint8_t, z1,
+ svluti2_lane_bf16 (z28, z0, 0),
+ svluti2_lane_bf16 (z28, z0, 0))
Following on from the comment above about these intrinsics being
overloaded: the second call above should not have a type suffix.
Similarly for the other tests (sorry!).
+
+/*
+** luti2_test_imm1:
+** luti2 z1\.h, \{ z28\.h \}, z0\[1\]
+** ret
+*/
+
+TEST_XN_SINGLE (luti2_test_imm1, svbfloat16_t, svuint8_t, z1,
+ svluti2_lane_bf16 (z28, z0, 1),
+ svluti2_lane_bf16 (z28, z0, 1))
+
+/*
+** luti2_test_tied:
+** luti2 z28\.h, \{ z28\.h \}, z0\[2\]
+** ret
+*/
+
+TEST_XN_SINGLE (luti2_test_tied, svbfloat16_t, svuint8_t, z28,
+ svluti2_lane_bf16 (z28, z0, 2),
+ svluti2_lane_bf16 (z28, z0, 2))
I think we should test the upper bound of each range. A combination of
testing the upper bound + the dg-do change would have caught the ? 1 : 7
thing above.
[...]
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index 45ba2f47a9d..e0d9867801c 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -4800,6 +4800,18 @@ proc check_effective_target_aarch64_sve2 { } {
}]
}
+# Return 1 if this is an AArch64 target supporting LUT (Lookup table)
+proc check_effective_target_aarch64_lut { } {
+ if { ![istarget aarch64*-*-*] || ![check_effective_target_aarch64_sve2] } {
+ return 0
+ }
+ return [check_no_compiler_messages aarch64_lut assembly {
+ #if !defined (__ARM_FEATURE_LUT)
+ #error FOO
+ #endif
+ }]
+}
+
# Return 1 if this is an AArch64 target only supporting SVE (not SVE2).
proc check_effective_target_aarch64_sve1_only { } {
return [expr { [check_effective_target_aarch64_sve]
I'm not sure this is needed.
Thanks,
Richard
