[PATCH] D14327: Add llvm.ldexp.* intrinsic, associated SDNode and library calls

[Matt Arsenault via llvm-commits]

arsenm added a comment.

This mostly LGTM except for the question of error behavior. There should be a few additions to get more of the benefits of using an intrinsic over a libcall.  ldexp should be added to isTriviallyVectorizable and isSafeToSpeculativelyExecute with appropriate tests, assuming we can assume it doesn't set errno. This could be a follow up patch.


================
Comment at: docs/LangRef.rst:9889
@@ +9888,3 @@
+power, thus returning the same values as the libm ``ldexp`` functions
+would, and handles error conditions in the same way.
+
----------------
I don't think this should be defined it to handling the same way as libm. I think we should say it does not set errno, and then to only do the libcall transformation if the call is marked readonly/readnone. This is an area that isn't handled particularly consistently by the existing math intrinsics.

================
Comment at: test/CodeGen/AMDGPU/llvm.ldexp.ll:21
@@ +20,3 @@
+}
+
+declare float @llvm.ldexp.f32(float, i32) #1
----------------
Should include vector versions for at least v2f32, v4f32 and v2f64

Also, can you merge the existing llvm.AMDGPU.ldexp.ll test into this one and rename them with a legacy_ prefix


http://reviews.llvm.org/D14327