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

[Nikita Popov via Phabricator via llvm-commits]

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

LGTM. I would simplify the MetadataAsValue cloning a bit, but it's not particularly important either.



================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:895
+  MDMap.clear();
+  MDVMap.clear();
 
----------------
These can be dropped (inlining performs only one clone).


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:927
+                                 MDMap[cast<MDNode>(I->getMetadata())])});
+  }
+}
----------------
I don't think it makes sense to separately track MDV and MDVMap. I would create the new MetadataAsValue directly in remap:

```
    if (auto *II = dyn_cast<IntrinsicInst>(I)) {
      if (II->getIntrinsicID() == Intrinsic::experimental_noalias_scope_decl) {
        auto *MV = cast<MetadataAsValue>(
            II->getOperand(Intrinsic::NoAliasScopeDeclScopeArg)));
        auto *NewMV = MetadataAsValue::get(
            I->getContext(), MDMap[cast<MDNode>(MV->getMetadata())]);
        II->setOperand(Intrinsic::NoAliasScopeDeclScopeArg, NewMV);
      }
    }
```

Or so.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1823
+    // this metadata needs to be cloned so that the inlined blocks
+    // have different "unique scopes" at every call site.
+    // Track the metadata that must be cloned. Do this before other changes to
----------------
The above part of the comment is repeated below. You might want to drop it in one place.


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


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

https://reviews.llvm.org/D93040