[PATCH] D50401: [GISel]: Add Opcodes for a few Libm Intrinsics

[Daniel Sanders via Phabricator via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

This LGTM. I'm ok with the G_INTRINSIC_TRUNC spelling for the trunc() intrinsic but we should reach an agreement on it before committing.



================
Comment at: include/llvm/Target/GenericOpcodes.td:516
+//------------------------------------------------------------------------------
+// LIBM Intrinsics
+//------------------------------------------------------------------------------
----------------
aemerson wrote:
> arsenm wrote:
> > aditya_nandakumar wrote:
> > > aemerson wrote:
> > > > Perhaps say "LIBM compatible intrinsics"
> > > Would you name it G_FTRUNC? I thought that is very similar to G_FPTRUNC and might lead to confusion. One other name I can think of is G_INTRINSIC_TRUNC which makes it explicit that this is the intrinsic equivalent in llvm IR. 
> > Yes
> Matt can you clarify which of those two suggestions you meant? Aditya named the intrinsics G_INTRINSIC_TRUNC but did you mean we should name them G_FTRUNC?
FWIW, I think we should make it clear in the name that it's the operation performed by trunc() (which isn't a type conversion) and not the type conversion operation performed by fptrunc from LLVM-IR.


================
Comment at: test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll:1412
+declare float @llvm.trunc.f32(float)
+define float @test_libm_trunc(float %a) {
+; CHECK-LABEL: name: test_libm_trunc
----------------
If we're dropping libm elsewhere we should probably drop it here too


================
Comment at: test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll:1422
+declare float @llvm.round.f32(float)
+define float @test_libm_round(float %a) {
+; CHECK-LABEL: name: test_libm_round
----------------
Likewise


https://reviews.llvm.org/D50401