[PATCH] D76792: [InlineFunction] Update metadata on loads that are return values

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Minor comments, otherwise LGTM.



================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1181
+      return dyn_cast_or_null<LoadInst>(VMap.lookup(V));
+    return nullptr;
+  };
----------------
Nit: Cheap condition first: `UpdateXXX && isa<...>`


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1226
       continue;
-    // Add to the existing attributes.
-    AttributeList AL = NewRetVal->getAttributes();
-    AttributeList NewAL =
-        AL.addAttributes(Context, AttributeList::ReturnIndex, AB);
-    NewRetVal->setAttributes(NewAL);
+    if (isa<CallBase>(NewRetVal)) {
+      // Add to the existing attributes.
----------------
`if (auto *CB = dyn_cast<CallBase>(...))`
should be simpler.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1236
+      // following return attributes on callsite only applies to pointer typed
+      // returns.
+      if (CS.isReturnNonNull())
----------------
Nit: I really hope that'll change, for non-null at least. Though, then we might also want to allow it in the metadata so all will be fine except the comment ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76792