rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGCoroutine.cpp:712
+ assert(CurFn);
+ CurFn->addFnAttr("coroutine.presplit", "0");
}
----------------
The assertion here is not necessary; if it was, we'd need it everywhere.
Please add a comment like "LLVM expects coroutines to have this attribute set
coming out of the frontend."
================
Comment at: llvm/docs/Coroutines.rst:1179
+The frontend should add attribute `"coroutine.presplit"` with value `"0"` for
the coroutine
+containing `coro.id`.
+
----------------
Please add this paragraph (modified appropriately) to all of the other
`coro.id.*` intrinsics.
================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:198
case Intrinsic::coro_id_async:
F.addFnAttr(CORO_PRESPLIT_ATTR, PREPARED_FOR_SPLIT);
break;
----------------
This should turn into the same assertion as above. (Swift will need a change,
but that's Swift's problem, and I'll take care of it.)
================
Comment at: llvm/lib/Transforms/Coroutines/CoroInternal.h:42
+// attributes since these attributes are already used outside the LLVM's
+// coroutine module.
#define CORO_PRESPLIT_ATTR "coroutine.presplit"
----------------
Editorial: don't put "the" before LLVM here
Since this change requires frontend cooperation, and the refactor will *also*
require frontend cooperation, should we just do this in one step so that
frontends are less disrupted?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115790/new/
https://reviews.llvm.org/D115790
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
