[PATCH] D93040: [InlineFunction] Use llvm.experimental.noalias.scope.decl for noalias arguments.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:833
+/// hold between the two clones, in which case it is necessary to clone the
+/// metadata using this utility. This comes up with inlining and unrolling.
+class ScopedAliasMetadataCloner {
----------------
The mention of unrolling here no longer makes sense.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:867
+                      ->getMetadata()))
+            MD.insert(M);
+    }
----------------
While this collects the metadata on the experimental.noalias.scope.decl, I'm not seeing any code that would actually update it on the cloned instructions. remap() only checks the noalias and alias.scope metadata, no?

Looking through the tests, I think we're missing a test case that inlines two levels, so that the first creates the intrinsic, and the second has to properly rename its operand.


================
Comment at: llvm/test/Transforms/Coroutines/coro-retcon-resume-values.ll:2
+; RUN: opt < %s -enable-coroutines -O2 -S | FileCheck %s --check-prefixes=CHECK,OPM
+; RUN: opt < %s -enable-coroutines -passes='default<O2>' -S | FileCheck %s --check-prefixes=CHECK,NPM
 
----------------
You need to add `-aa-pipeline=default` to avoid the NPM regression.


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

https://reviews.llvm.org/D93040