[PATCH] D110288: Move pow transformations to sqrt/cbrt to earlier in the compiler pipeline

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

@efriedma is this what you had in mind (based on your comments in https://reviews.llvm.org/D101759)?

Seems like a reasonable approach to me, but it would affect many targets (and the noopt codegen it seems), so I'm not sure if we would need TTI hooks to help us make target dependent decisions here. @spatel @nemanjai any thoughts?



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1695
+      Value *Sqrt = nullptr, *Cbrt = nullptr, *Pow23 = nullptr;
+      Value *SqrtTmp = nullptr, *SqrtSqrt = nullptr, *Pow075 = nullptr;
       if (!ExpoA.isInteger()) {
----------------
declare `SqrtTmp` inside the only block that uses it.


================
Comment at: llvm/test/CodeGen/AArch64/pow.75.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -O3 -S -mtriple=aarch64-- %s 2>&1 | FileCheck %s
 ; REQUIRES: asserts
----------------
Is -O3 really the minimum opt level that would trigger this transformation?


================
Comment at: llvm/test/CodeGen/AArch64/pow.75.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -O3 -S -mtriple=aarch64-- %s 2>&1 | FileCheck %s
 ; REQUIRES: asserts
----------------
bmahjour wrote:
> Is -O3 really the minimum opt level that would trigger this transformation?
Also, these lit tests no longer test codegen, so they should probably be moved elsewhere... under Transforms subdirectories.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110288