[PATCH] D139295: [Coroutines] Don't mark the parameter attribute of resume function as noalias

[Nikita Popov via Phabricator via llvm-commits]

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

LG



================
Comment at: clang/test/CodeGenCoroutines/pr59221.cpp:85
+
+// Checks that the store for the result value 42 is not mitoptimized out.
+// CHECK: define{{.*}}_Z3foov(
----------------
mitoptimized -> misoptimized?


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:849
   ParamAttrs.addAttribute(Attribute::NonNull);
-  ParamAttrs.addAttribute(Attribute::NoAlias);
+  ParamAttrs.addAttribute(Attribute::NoUndef);
+  /// FIXME: Is it really good to add the NoAlias attribute?
----------------
ChuanqiXu wrote:
> The documentation says the `NonNull` attribute should be used in the combination of `NoUndef`.
dereferenceable implies noundef, but I guess it doesn't hurt to be explicit about it.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:850
+  ParamAttrs.addAttribute(Attribute::NoUndef);
+  /// FIXME: Is it really good to add the NoAlias attribute?
+  if (NoAlias)
----------------
ChuanqiXu wrote:
> Although I suspect the Retcon ABI has the similar problem, this maintains the same behavior with Retcon ABI.
I'd move this FIXME to the call for the retcon ABI.


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

https://reviews.llvm.org/D139295