[PATCH] D82820: [InstCombine] Fix mismatched attribute lists for combined calls
Gui Andrade via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 1 14:04:22 PDT 2020
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
More information about the llvm-commits
mailing list