[llvm-commits] [PATCH] Instcombine: Don't optimize pow(2.0, x) -> exp2(x) if exp2 is unavailable

Meador Inge meadori at codesourcery.com
Mon Nov 26 17:53:00 PST 2012


On Nov 26, 2012, at 5:25 PM, Derek Schuff wrote:

> Right, let me give a little bit more background.
> I'm building newlib to use as a C library for a platform (Native
> Client). In this case the notion of 'the target has exp2' is a little
> funny because we are building the target platform. However, just
> building the C library with -fno-builtin-foo doesn't quite work
> because these functions are also seen again by the optimizer during
> LTO (and it appears that clang doesn't support that yet anyway).  I
> had in mind to just set exp2 as unavailable for Native Client in TLI
> since even though it would be ok to do it for user code, it would
> still be a pessimization. Independently of what I want to do for
> native client, the patch I sent here seems to fix a legitimate bug
> which would trigger on any platform that lacked exp2.
> As for testing, adding it to disable-simplify-libcalls.ll isn't quite
> sufficient because if you disable all libcall simplification (as that
> test does) you get the right result.

Upstream newlib builds with -fno-builtin.  Does the copy of newlib you
are working with disable that?  Although, I guess, the LTO part you
describe above makes that moot anyway.  LLVM LTO doesn't respect the
original options something was compiled with?

> --- a/lib/Target/TargetLibraryInfo.cpp
> +++ b/lib/Target/TargetLibraryInfo.cpp
> @@ -375,6 +375,13 @@ static void initialize(TargetLibraryInfo &TLI, const Triple &T,
>    default:
>      TLI.setUnavailable(LibFunc::ffsll);
>    }
> +
> +  if (T.getOS() == Triple::NativeClient) {
> +    // Newlib's exp2f implementation just calls powf, so prevent the pow
> +    // call from being turned back into an exp2 call
> +    TLI.setUnavailable(LibFunc::exp2);
> +    TLI.setUnavailable(LibFunc::exp2f);
> +  }
>  }

As you mentioned this is a pessimization.  You will get the result you want for the pow
simplification, but it also completely disables the exp2 and exp2f simplifications
even though implementations for those library functions are actually available.  This
seems a little hacky to me.  Building libc with -fno-builtin seems like a better approach,
but it sounds like that isn't working.

> diff --git a/lib/Transforms/Utils/SimplifyLibCalls.cpp b/lib/Transforms/Utils/SimplifyLibCalls.cpp
> index 27ccdc1..5641837 100644
> --- a/lib/Transforms/Utils/SimplifyLibCalls.cpp
> +++ b/lib/Transforms/Utils/SimplifyLibCalls.cpp
> @@ -1119,7 +1119,10 @@ struct PowOpt : public UnsafeFPLibCallOptimization {
>      if (ConstantFP *Op1C = dyn_cast<ConstantFP>(Op1)) {
>        if (Op1C->isExactlyValue(1.0))  // pow(1.0, x) -> 1.0
>          return Op1C;
> -      if (Op1C->isExactlyValue(2.0))  // pow(2.0, x) -> exp2(x)
> +      if (Op1C->isExactlyValue(2.0) &&
> +          TLI->has(FT->getParamType(0)->isDoubleTy() ?
> +                   LibFunc::exp2 : LibFunc::exp2f))
> +        // pow(2.0, x) -> exp2(x)
>          return EmitUnaryFloatFnCall(Op2, "exp2", B, Callee->getAttributes());
>      }

This bit looks fine.

> --- a/test/Transforms/InstCombine/disable-simplify-libcalls.ll
> +++ b/test/Transforms/InstCombine/disable-simplify-libcalls.ll
> @@ -49,6 +49,7 @@ declare i64 @labs(i64)
>  declare i64 @llabs(i64)
>  declare i32 @printf(i8*)
>  declare i32 @sprintf(i8*, i8*)
> +declare double @pow(double, double)
>  
>  define double @t1(double %x) {
>  ; CHECK: @t1
> @@ -333,3 +334,10 @@ define void @t37(i8* %x) {
>    ret void
>  ; CHECK: call i32 @sprintf
>  }
> +
> +define double @t38(double %x) {
> +; CHECK: @t38
> +  %ret = call double @pow(double 2.0, double %x)
> +  ret double %ret
> +; CHECK: call double @pow
> +}


As you pointed out this test isn't related to the change your are making.
It is a fine addition to the test suite, but should be a different patch.

> --- /dev/null
> +++ b/test/Transforms/InstCombine/simplify-libcalls-nacl.ll
> @@ -0,0 +1,20 @@
> +; Test that simplify-libcalls does not simplify
> +;
> +; RUN: opt < %s -mtriple=le32-unknown-nacl -instcombine -S | FileCheck %s
> +
> +declare double @pow(double, double)
> +declare float @powf(float, float)
> +
> +define double @t1(double %x) {
> +; CHECK: @t1
> +  %ret = call double @pow(double 2.0, double %x)
> +  ret double %ret
> +; CHECK: call double @pow
> +}
> +
> +define float @t2(float %x) {
> +; CHECK: @t2
> +  %ret = call float @powf(float 2.0, float %x)
> +  ret float %ret
> +; CHECK: call float @powf
> +}

This looks OK too.  Just put the test in pow-1.ll, though.  You can add
something like the following to only run it for a platform that doesn't
have exp2:

; RUN: opt < %s -mtriple i386-pc-win32 -instcombine -S | FileCheck %s -check-prefix=NOEXP2

…

; CHECK-NOEXP2: @t1
…
; CHECK-NOEXP2: call float @powf

--
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software





More information about the llvm-commits mailing list