On Fri, Jul 13, 2018 at 04:15:41AM -0500, Richard Sandiford wrote:
> Given a pattern like:
>
> (define_insn "aarch64_frecpe<mode>" ...)
>
> the SVE ACLE implementation wants to generate the pattern for a
> particular (non-constant) mode. This patch automatically generates
> helpers to do that, specifically:
>
> // Return CODE_FOR_nothing on failure.
> insn_code maybe_code_for_aarch64_frecpe (machine_mode);
>
> // Assert that the code exists.
> insn_code code_for_aarch64_frecpe (machine_mode);
>
> // Return NULL_RTX on failure.
> rtx maybe_gen_aarch64_frecpe (machine_mode, rtx, rtx);
>
> // Assert that generation succeeds.
> rtx gen_aarch64_frecpe (machine_mode, rtx, rtx);
>
> Many patterns don't have sensible names when all <...>s are removed.
> E.g. "<optab><mode>2" would give a base name "2". The new functions
> therefore require explicit opt-in, which should also help to reduce
> code bloat.
>
> The (arbitrary) opt-in syntax I went for was to prefix the pattern
> name with '@', similarly to the existing '*' marker.
>
> The patch also makes config/aarch64 use the new routines in cases where
> they obviously apply. This was mostly straight-forward, but it seemed
> odd that we defined:
>
> aarch64_reload_movcp<...><P:mode>
>
> but then only used it with DImode, never SImode. If we should be
> using Pmode instead of DImode, then that's a simple change,
> but should probably be a separate patch.
>
> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
> and x86_64-linux-gnu. I think I can self-approve the gen* bits,
> but OK for the AArch64 parts?
For what it is worth, I like the change to AArch64, and would support it
when you get consensus around the new syntax from other targets.
You only have to look at something like:
> - rtx (*gen) (rtx, rtx, rtx);
> -
> - switch (src_mode)
> - {
> - case E_V8QImode:
> - gen = gen_aarch64_simd_combinev8qi;
> - break;
> - case E_V4HImode:
> - gen = gen_aarch64_simd_combinev4hi;
> - break;
> - case E_V2SImode:
> - gen = gen_aarch64_simd_combinev2si;
> - break;
> - case E_V4HFmode:
> - gen = gen_aarch64_simd_combinev4hf;
> - break;
> - case E_V2SFmode:
> - gen = gen_aarch64_simd_combinev2sf;
> - break;
> - case E_DImode:
> - gen = gen_aarch64_simd_combinedi;
> - break;
> - case E_DFmode:
> - gen = gen_aarch64_simd_combinedf;
> - break;
> - default:
> - gcc_unreachable ();
> - }
> -
> - emit_insn (gen (dst, src1, src2));
> + emit_insn (gen_aarch64_simd_combine (src_mode, dst, src1, src2));
To understand this is a Good Thing for code maintainability.
Thanks,
James
>
> Any objections to this approach or syntax?
>
> Richard
