This patch enables folding following instructions under -ffast-math flag: log10(pow(10.0,x)) -> x, log2(pow(2.0,x)) -> x
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 27143 Build 27142: arc lint + arc unit
Event Timeline
Seems ok.
lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
4944 | Hmm, do we want to canonicalize Intrinsic::pow(2.0, x) to exp2(x) instead ? |
lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
4944 | If Intrinsic::pow(2.0, x) to exp2(x) transform is useful somewhere else or you have strong opinion about it we do. Otherwise I don't see the reason for this. |
lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
4944 | It's a specialized form of the more general math function, so we can assume it has a faster implementation. I think we should add that fold and reduce this patch. |
lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
4944 | There is a counter-argument to that: (that i have thought of after asking, but did not post) I.e. is there some situation where we might end up with simplifyUnaryIntrinsic() being called |
lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
4944 | Instsimplify may be used independently of instcombine by other passes, so yes, it is possible that the non-canonical form will be seen here. It's not generally a concern with the typical optimization pipeline since we run instcombine...a lot. So the full solution is: leave this patch as-is (handle both forms) and add the instcombine too. |
LG unless others have more comments (@spatel ?)
You probably know this already, but there are some other low-hanging fruit here, e.g.
https://godbolt.org/z/Trdf_e
https://www.wolframalpha.com/input/?i=log2(x)+%2F+log2(10)+%3D+log10(x)
Thanks, I'll dive into it. Btw, pow(2.0, x) --> exp2(x) is already implemented, see https://godbolt.org/z/UADcWe.
If you're interested in math call transforms, you might also want to look at:
https://bugs.llvm.org/show_bug.cgi?id=40541
One more comment: 'reassoc' is generally enough to go crazy with FP, but we might also want to require 'nsz' on this and related transforms to be safer because:
x = -0.0
exp2(x) = 1.0
log2(exp2(x)) = 0.0 <-- output flipped sign from input
Hmm, do we want to canonicalize Intrinsic::pow(2.0, x) to exp2(x) instead ?