Tamar Christina (tnfchris) <[email protected]> requested changes to the code:
Looks pretty good, just some minor changes.
> +++ gcc/config/aarch64/aarch64-neon-builtins-base.cc
> @@ -0,0 +302,4 @@
> +};
> +
> +struct gimple_dup_lane : public gimple_function_base
> +{
Doesn't this also need
```
if (optimize == 0)
return nullptr;
```
> +++ gcc/config/aarch64/aarch64-simd.md
> @@ -9854,3 +9855,4 @@
> [(set_attr "type" "crypto_sha3")]
> )
>
> +(define_insn "aarch64_rax1qv2di"
this one isn't needed, just make the `*aarch64_rax1qv2di` above not anonymous.
> +++ gcc/testsuite/gcc.target/aarch64/sme/inlining_10.c
> @@ -21,1 +20,3 @@
> -call_vbsl () // { dg-error "inlining failed" }
> +// Gets expanded to bitwise select early, so no error. An error would be
> +// more correct though.
> +inline void __attribute__ ((always_inline))
I think BSL was chosen here not because we wanted to test `bsl` itself but
because we wanted to test the inlining behavior of a non-lowered intrinsics.
Now that you lower `bsl` instead of removing the error you should pick another
intrinsics that isn't lowered. Otherwise we both checks here doesn't check
inlining errors.
Same with the below.
> +++ gcc/testsuite/gcc.target/aarch64/neon/aarch64-neon.exp
> @@ -0,0 +33,4 @@
> +
> +# Main loop.
> +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*\[cCs\]]] \
> + " -ansi -pedantic-errors -std=c23 -O3 -march=armv8-a+simd" ""
I think we just want -O2 here, since that's the standard compile flag for most.
Or even -O1 since intrinsics shouldn't rely on optimizations to produce the
result stated by ACLE.
> +++ gcc/config/aarch64/aarch64-neon-builtins-shapes.cc
> @@ -0,0 +108,4 @@
> + check_fn_t m_check_fn1;
> + check_fn_t m_check_fn2;
> +
> + void build (function_builder &b,
In the removed code above the comment said
```
case UNSPEC_VEC_COPY:
/* & rather than && so that we report errors against both indices. */
return (require_immediate_lane_index (1, 0)
& require_immediate_lane_index (3, 2));
```
and now we only report the first error. I think we should restore that
behavior, so just use `&` instead of `&&` here to report all errors at the same
time.
--
https://forge.sourceware.org/gcc/gcc-TEST/pulls/158#issuecomment-6435
