rdapp....@gmail.com writes:
> From: Robin Dapp <rd...@ventanamicro.com>
>
> This adds zero else operands to masked loads and their intrinsics.
> I needed to adjust more than initially thought because we rely on
> combine for several instructions and a change in a "base" pattern
> needs to propagate to all those.
>
> For the lack of a better idea I used a function call property to specify
> whether a builtin needs an else operand or not. Somebody with better
> knowledge of the aarch64 target can surely improve that.
This part no longer holds :)
> [...]
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> index fe16d93adcd..406ceb13a4c 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc
> [...]
> @@ -1537,11 +1538,14 @@ public:
> {
> insn_code icode;
> if (e.vectors_per_tuple () == 1)
> - icode = convert_optab_handler (maskload_optab,
> - e.vector_mode (0), e.gp_mode (0));
> + {
> + icode = convert_optab_handler (maskload_optab,
> + e.vector_mode (0), e.gp_mode (0));
> + e.args.quick_push (CONST0_RTX (e.vector_mode (0)));
It looks like this should no longer be necessary, since
use_contiguous_load_insn should add it instead. (Let me know if that's
wrong though.)
> + }
> else
> icode = code_for_aarch64 (UNSPEC_LD1_COUNT, e.tuple_mode (0));
> - return e.use_contiguous_load_insn (icode);
> + return e.use_contiguous_load_insn (icode, true);
> }
> };
>
> @@ -1551,13 +1555,19 @@ class svld1_extend_impl : public extending_load
> public:
> using extending_load::extending_load;
>
> + unsigned int
> + call_properties (const function_instance &) const override
> + {
> + return CP_READ_MEMORY;
> + }
> +
It looks like this is a left-over from the previous version and
could be removed.
> rtx
> expand (function_expander &e) const override
> {
> - insn_code icode = code_for_aarch64_load (UNSPEC_LD1_SVE, extend_rtx_code
> (),
> + insn_code icode = code_for_aarch64_load (extend_rtx_code (),
> e.vector_mode (0),
> e.memory_vector_mode ());
> - return e.use_contiguous_load_insn (icode);
> + return e.use_contiguous_load_insn (icode, true);
> }
> };
>
> [...]
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc
> b/gcc/config/aarch64/aarch64-sve-builtins.cc
> index ef14f8cd39d..84c0a0caa50 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> @@ -4229,7 +4229,7 @@ function_expander::use_vcond_mask_insn (insn_code icode,
> Extending loads have a further predicate (operand 3) that nominally
> controls the extension. */
> rtx
> -function_expander::use_contiguous_load_insn (insn_code icode)
> +function_expander::use_contiguous_load_insn (insn_code icode, bool has_else)
The comment should describe the new parameter. Maybe add a new paragraph:
HAS_ELSE is true if the pattern has an additional operand that specifies
the values of inactive lanes. This exists to match the general maskload
interface and is always zero for AArch64. */
> {
> machine_mode mem_mode = memory_vector_mode ();
>
> [...]
> diff --git a/gcc/config/aarch64/aarch64-sve.md
> b/gcc/config/aarch64/aarch64-sve.md
> index 06bd3e4bb2c..a2e9f52d024 100644
> --- a/gcc/config/aarch64/aarch64-sve.md
> +++ b/gcc/config/aarch64/aarch64-sve.md
> [...]
> @@ -1302,11 +1303,14 @@ (define_expand "vec_load_lanes<mode><vsingle>"
> [(set (match_operand:SVE_STRUCT 0 "register_operand")
> (unspec:SVE_STRUCT
> [(match_dup 2)
> - (match_operand:SVE_STRUCT 1 "memory_operand")]
> + (match_operand:SVE_STRUCT 1 "memory_operand")
> + (match_dup 3)
> + ]
Formatting nit, sorry, but: local style is to put the closing ]
on the previous line.
OK with those changes. Thanks a lot for doing this!
Richard
> UNSPEC_LDN))]
> "TARGET_SVE"
> {
> operands[2] = aarch64_ptrue_reg (<VPRED>mode);
> + operands[3] = CONST0_RTX (<MODE>mode);
> }
> )
>
> [...]
