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

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:519-525
-  if (Shape.ABI != coro::ABI::Async)
-    NewF->addParamAttr(0, Attribute::NonNull);
-
-  // For the async lowering ABI we can't guarantee that the context argument is
-  // not access via a different pointer not based on the argument.
-  if (Shape.ABI != coro::ABI::Async)
-    NewF->addParamAttr(0, Attribute::NoAlias);
----------------
It is dead. It would be covered by the following change.


================
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?
----------------
The documentation says the `NonNull` attribute should be used in the combination of `NoUndef`.


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


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

https://reviews.llvm.org/D139295