[PATCH] D151943: [InstCombine] Propagate some func/arg/ret attributes from caller to callsite

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

> I somewhat agree although I'm not sure where else? I thought about putting it in the attributor but 1) the attributor is not enabled (and its not clear when/if that will happen) and 2) the attributor seems to be for IPO which doesn't really describe this well imo. Figured since its an exact transform on the specific instructions for the sake of other optimization passes instcombine fits the mold reasonable well.

FunctionAttrs seems like a reasonable place to put it?



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3652
+  if (const BasicBlock *BB = Call.getParent()) {
+    if (const Function *PF = BB->getParent()) {
+
----------------
Both of these cannot be null in this context.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3671
+      std::array CallerFnMemAttrPropagations = {
+          Attribute::ReadNone, Attribute::ReadOnly, Attribute::WriteOnly};
+
----------------
These attributes only exist on arguments, not functions.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3725
+      // callsite's return value if it is used as the return value of the
+      // caller.
+      std::array CallerReturnAttrPropagations = {Attribute::NoUndef,
----------------
This is incorrect for UB-implying attributes like noundef, if the call is not guaranteed to transfer to the return.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3729
+      if (!Call.getType()->isVoidTy() && !PF->getType()->isVoidTy() &&
+          BB->getTerminator() && isa<ReturnInst>(BB->getTerminator())) {
+        if (cast<ReturnInst>(BB->getTerminator())->getReturnValue() == &Call) {
----------------
BB->getTerminator() cannot be null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151943