[PATCH] D129989: [AArch64] Add f16 fpimm patterns

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:1629
+def : Pat<(f16 fpimm:$in),
+  (COPY_TO_REGCLASS (MOVi32imm (bitcast_fpimm_to_i32 f16:$in)), FPR16)>;
 def : Pat<(f32 fpimm:$in),
----------------
dmgreen wrote:
> COPY's are quite loose in their definition, but it is probably best to keep them copying between values of the same size. I think this can use FMOVWHr directly, so long as the instruction has already been defined.
> It should probably have a: `let Predicates = [HasFullFP16]` too.
Is `HasFullFP16` relevant? The lowering requires no special instructions so I'd even argue the `setOperationAction` call requires no protection.  What's important is whether the `f16` type is legal.  Once we get to operation lowering and isel we only need to protect instructions not types.

We made this mistake for `bf16` for SVE and it just lead to unnecessary complexity which was eventually removed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129989/new/

https://reviews.llvm.org/D129989