Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes:
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> index c24c0548724..1ef4ea2087b 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> @@ -44,6 +44,14 @@
> #include "aarch64-sve-builtins-shapes.h"
> #include "aarch64-sve-builtins-base.h"
> #include "aarch64-sve-builtins-functions.h"
> +#include "aarch64-builtins.h"
> +#include "gimple-ssa.h"
> +#include "tree-phinodes.h"
> +#include "tree-ssa-operands.h"
> +#include "ssa-iterators.h"
> +#include "stringpool.h"
> +#include "value-range.h"
> +#include "tree-ssanames.h"
Minor, but: I think the preferred approach is to include "ssa.h"
rather than include some of these headers directly.
>
> using namespace aarch64_sve;
>
> @@ -1207,6 +1215,56 @@ public:
> insn_code icode = code_for_aarch64_sve_ld1rq (e.vector_mode (0));
> return e.use_contiguous_load_insn (icode);
> }
> +
> + gimple *
> + fold (gimple_folder &f) const OVERRIDE
> + {
> + tree arg0 = gimple_call_arg (f.call, 0);
> + tree arg1 = gimple_call_arg (f.call, 1);
> +
> + /* Transform:
> + lhs = svld1rq ({-1, -1, ... }, arg1)
> + into:
> + tmp = mem_ref<int32x4_t> [(int * {ref-all}) arg1]
> + lhs = vec_perm_expr<tmp, tmp, {0, 1, 2, 3, ...}>.
> + on little endian target. */
> +
> + if (!BYTES_BIG_ENDIAN
> + && integer_all_onesp (arg0))
> + {
> + tree lhs = gimple_call_lhs (f.call);
> + auto simd_type = aarch64_get_simd_info_for_type (Int32x4_t);
Does this work for other element sizes? I would have expected it
to be the (128-bit) Advanced SIMD vector associated with the same
element type as the SVE vector.
The testcase should cover more than just int32x4_t -> svint32_t,
just to be sure.
> +
> + tree elt_ptr_type
> + = build_pointer_type_for_mode (simd_type.eltype, VOIDmode, true);
> + tree zero = build_zero_cst (elt_ptr_type);
> +
> + /* Use element type alignment. */
> + tree access_type
> + = build_aligned_type (simd_type.itype, TYPE_ALIGN (simd_type.eltype));
> +
> + tree tmp = make_ssa_name_fn (cfun, access_type, 0);
> + gimple *mem_ref_stmt
> + = gimple_build_assign (tmp, fold_build2 (MEM_REF, access_type, arg1,
> zero));
Long line. Might be easier to format by assigning the fold_build2 result
to a temporary variable.
> + gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT);
> +
> + tree mem_ref_lhs = gimple_get_lhs (mem_ref_stmt);
> + tree vectype = TREE_TYPE (mem_ref_lhs);
> + tree lhs_type = TREE_TYPE (lhs);
Is this necessary? The code above supplied the types and I wouldn't
have expected them to change during the build process.
> +
> + int source_nelts = TYPE_VECTOR_SUBPARTS (vectype).to_constant ();
> + vec_perm_builder sel (TYPE_VECTOR_SUBPARTS (lhs_type), source_nelts, 1);
> + for (int i = 0; i < source_nelts; i++)
> + sel.quick_push (i);
> +
> + vec_perm_indices indices (sel, 1, source_nelts);
> + gcc_checking_assert (can_vec_perm_const_p (TYPE_MODE (lhs_type),
> indices));
> + tree mask = vec_perm_indices_to_tree (lhs_type, indices);
> + return gimple_build_assign (lhs, VEC_PERM_EXPR, mem_ref_lhs,
> mem_ref_lhs, mask);
Nit: long line.
> + }
> +
> + return NULL;
> + }
> };
>
> class svld1ro_impl : public load_replicate
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index f650abbc4ce..47810fec804 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -23969,6 +23969,35 @@ aarch64_evpc_sve_tbl (struct expand_vec_perm_d *d)
> return true;
> }
>
> +/* Try to implement D using SVE dup instruction. */
> +
> +static bool
> +aarch64_evpc_sve_dup (struct expand_vec_perm_d *d)
> +{
> + if (BYTES_BIG_ENDIAN
> + || d->perm.length ().is_constant ()
> + || !d->one_vector_p
> + || d->target == NULL
> + || d->op0 == NULL
These last two lines mean that we always return false for d->testing.
The idea instead is that the return value should be the same for both
d->testing and !d->testing. The difference is that for !d->testing we
also emit code to do the permute.
> + || GET_MODE_NUNITS (GET_MODE (d->target)).is_constant ()
Sorry, I've forgotten the context now, but: these positive tests
for is_constant surprised me. Do we really only want to do this
for variable-length SVE code generation, rather than fixed-length?
> + || !GET_MODE_NUNITS (GET_MODE (d->op0)).is_constant ())
> + return false;
> +
> + if (d->testing_p)
> + return true;
This should happen after the later tests, once we're sure that the
permute vector has the right form. If the issue is that op0 isn't
provided for testing then I think the hook needs to be passed the
input mode alongside the result mode.
It might then be better to test:
aarch64_classify_vector_mode (...input_mode...) == VEC_ADVSIMD
(despite what I said earlier, about testing is_constant, sorry).
> +
> + int npatterns = d->perm.encoding ().npatterns ();
> + if (!known_eq (npatterns, GET_MODE_NUNITS (GET_MODE (d->op0))))
> + return false;
> +
> + for (int i = 0; i < npatterns; i++)
> + if (!known_eq (d->perm[i], i))
> + return false;
> +
> + aarch64_expand_sve_dupq (d->target, GET_MODE (d->target), d->op0);
> + return true;
> +}
> +
> /* Try to implement D using SVE SEL instruction. */
>
> static bool
> @@ -24129,7 +24158,12 @@ aarch64_expand_vec_perm_const_1 (struct
> expand_vec_perm_d *d)
> else if (aarch64_evpc_reencode (d))
> return true;
> if (d->vec_flags == VEC_SVE_DATA)
> - return aarch64_evpc_sve_tbl (d);
> + {
> + if (aarch64_evpc_sve_dup (d))
> + return true;
> + else if (aarch64_evpc_sve_tbl (d))
> + return true;
> + }
> else if (d->vec_flags == VEC_ADVSIMD)
> return aarch64_evpc_tbl (d);
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463.c
> b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463.c
> new file mode 100644
> index 00000000000..35100a9e01c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#include "arm_neon.h"
> +#include "arm_sve.h"
> +
> +svint32_t f1 (int32x4_t x)
> +{
> + return svld1rq (svptrue_b8 (), &x[0]);
> +}
> +
> +svint32_t f2 (int *x)
> +{
> + return svld1rq (svptrue_b8 (), x);
> +}
> +
> +/* { dg-final { scan-assembler-times {\tdup\tz[0-9]+\.q, z[0-9]+\.q\[0\]} 2
> { target aarch64_little_endian } } } */
