[gen/AArch64] Generate helpers for substituting iterator values into pattern names

[James Greenhalgh]

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