[PATCH] D82820: [InstCombine] Fix mismatched attribute lists for combined calls

[Gui Andrade via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1459
+          .addAttributes(B.getContext(), AttributeList::FirstArgIndex + 0,
+                         Attrs.getAttributes(AttributeList::FirstArgIndex + 1));
   Module *Mod = Pow->getModule();
----------------
eugenis wrote:
> When transforming pow(exp(x), y) to exp(x *y), why is it the right behavior to copy the attributes from the "y" parameter, and drop the ones on "x" and on "exp(x)"? Why not union or intersection?
> 
> It feels like the safe default behavior should be to drop all attributes, and adds the ones that we know apply here, like "noundef".
You're definitely right, in the case of combining the pow parameters it doesn't really make sense to assign attributes from one or the other... maybe intersection makes sense though. I do still think it makes sense to copy attributes from `x` when we're eliminating a constant, like in `pow(10.0, x) -> exp10(x)`. So I've split them up into two different attribute lists.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82820