[PATCH] D28405: SimplifyLibCalls: Replace fabs libcalls with intrinsics

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Canonicalizing `@fabs() -> @llvm.fabs()` makes sense.

I'd like to see a regression test which specifically checks the fabs/fabsf/fabsl -> llvm.fabs transform.  Please include testcases which transform fabsl -> llvm.fabs.f64, fabsl -> llvm.fabs.f80, and fabsl -> llvm.fabs.f128, to make sure they all work as expected.

Please make sure to kill off all the existing optimizations besides this one which check for LibFunc::fabs or the string "fabsf" in a followup.



================
Comment at: test/Transforms/InstCombine/double-float-shrink-2.ll:26
+
+; This is replaced with the intrinsic, which is replacable.
+; DONT-SIMPLIFY: call float @llvm.fabs.f32(
----------------
"replacable" is a little unclear... I think the point is that the intrinsic does the right thing on all platforms?


================
Comment at: test/Transforms/InstCombine/fabs.ll:16
 ; CHECK-NEXT: %mul = fmul float %x, %x
-; CHECK-NEXT: %fabsf = tail call float @fabsf(float %mul)
+; CHECK-NEXT: %fabsf = call float @llvm.fabs.f32(float %mul)
 ; CHECK-NEXT: ret float %fabsf
----------------
This is probably going to fail on Windows... maybe change the test to call llvm.fabs rather than fabs?


https://reviews.llvm.org/D28405