[PATCH] D128591: Transforms: refactor pow(x, n) expansion where n is a constant integer value

[Paul Osmialowski via Phabricator via llvm-commits]

pawosm01 marked 2 inline comments as done.
pawosm01 added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1917
 
   // pow(x, n) -> x * x * x * ...
   const APFloat *ExpoF;
----------------
spatel wrote:
> Delete stale code comment.
OK.
Also followed the comment below.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1950
 
+    // pow(x, n) -> powi(x, n) if n is a constant signed integer value
     APSInt IntExpo(TLI->getIntSize(), /*isUnsigned=*/false);
----------------
spatel wrote:
> Add another line for the sqrt variant like:
>  // pow(x, n) -> powi(x, n) * sqrt(x) if n has exactly a 0.5 fraction
OK


================
Comment at: llvm/test/CodeGen/AArch64/powi.ll:7
+
+define float @powi_f32(float %x) nounwind {
+; CHECK-LABEL: powi_f32:
----------------
spatel wrote:
> These tests results are independent of this patch, so this could be committed as a preliminary "NFC" patch. 
> 
> We intentionally don't have regression tests that check end-to-end results of IR optimization + codegen because we want focused/unit testing at this level. The end-to-end tests live in test-suite instead (and could begin from C source too).
I'd keep it if it's not very disturbing. This is the last thing that links us to the idea that brought us here.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128591