[PATCH] D127158: [AArch64] Add intrinsic support for gpr<->fpr flavors of fixed-point converts

[Eli Friedman via Phabricator via llvm-commits]

efriedma added subscribers: fpetrogalli, efriedma.
efriedma added a comment.

I agree that the LLVM intrinsics should convert between the exact types listed in the signature of the intrinsic, but we need to make sure we still have an intrinsic that produces "scvtf h0, h0, #16" etc.

This patch also seems like it's changing a few too many things at once... can you split the patch into pieces that are more easy to review?

For something like "llvm.aarch64.neon.vcvtfp2fxs.i32.f32", I guess there are two possible instructions with effectively equivalent semantics.  Ideally, the compiler is just clever enough to figure out the best one from the context.  Given the way SelectionDAG isel works, you probably have to do some sort of post-isel fixup like AArch64AdvSIMDScalarPass .

In terms of what clang generates, we have to follow the NEON spec; we can't use an instruction that produces a different result just because it's more useful.  So if the spec says the conversion produces a 16-bit integer, we have to produce a 16-bit integer.  That said, it's a little suspicious that the spec says vcvth_n_s16_f16, vcvth_n_s32_f16, and vcvth_n_s64_f16 all produce exactly the same instruction; maybe there's a spec bug?  @dmgreen @fpetrogalli can you confirm whether the spec is right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127158