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

[Jeroen Dobbelaere via Phabricator via llvm-commits]

jeroen.dobbelaere added a comment.

Thanks for the review ! I'll try to come with an update later today.



================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:867
+                      ->getMetadata()))
+            MD.insert(M);
+    }
----------------
nikic wrote:
> 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.
Good catch, it indeed seems that I missed the remapping of the 'llvm.experimental.noalias.scope.decl' argument in this version :(

More tests is always better. I'll add one for this, and also one for the caller == callee case.



================
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
 
----------------
nikic wrote:
> You need to add `-aa-pipeline=default` to avoid the NPM regression.
That seems to do the trick.


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

https://reviews.llvm.org/D93040