[PATCH] D102353: [AArch64] Lower fpto*i.sat intrinsics.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/round-fptosi-sat-scalar.ll:43
+  %r = call half @llvm.floor.f16(half %a) nounwind readnone
+  %i = call i64 @llvm.fptosi.sat.i64.f16(half %r)
+  ret i64 %i
----------------
jbramley wrote:
> SjoerdMeijer wrote:
> > I haven't checked, but we don't have a f16 -> i32 variant of this?
> Isn't that `@llvm.fptosi.sat.i32.f16`, in `@testmswh`, above?
Yep, sorry, missed that!


================
Comment at: llvm/test/CodeGen/AArch64/round-fptosi-sat-scalar.ll:194
+entry:
+  %r = call half @llvm.trunc.f16(half %a) nounwind readnone
+  %i = call i32 @llvm.fptosi.sat.i32.f16(half %r)
----------------
jbramley wrote:
> SjoerdMeijer wrote:
> > A trunc from a f16 to a f16 should be a no-op? Do we need this? I see similar patterns below, so I must be missing something...
> We need it because `trunc` [truncates to an integer](https://llvm.org/docs/LangRef.html#llvm-trunc-intrinsic), leaving the result in the same FP format (like `frintz` in the fallback machine instruction sequence). It's not a no-op.
> 
> All of the tests in this file are round + convert sequences, like `round.conv.ll`. I added other tests to `fptosi-sat-scalar.ll` (etc) for the simple, standalone conversions.
Ah okay, thanks, and I see now. I read this too quickly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102353