ChuanqiXu marked 2 inline comments as done.
ChuanqiXu added inline comments.
================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:198
case Intrinsic::coro_id_async:
F.addFnAttr(CORO_PRESPLIT_ATTR, PREPARED_FOR_SPLIT);
break;
----------------
rjmccall wrote:
> 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.)
I don't add the assertion here since I am worrying it may break some builds.
Would you be happy to add the assertion when you change swift?
================
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"
----------------
rjmccall wrote:
> 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?
I didn't image swift before when I wrote the FIXME. In case we only need to
care about clang and mlir, I think it doesn't matter a lot. But it matters if
there is other frontends.
I also found that the values "UNPREPARED_FOR_SPLIT"，“PREPARED_FOR_SPLIT” and
"ASYNC_RESTART_AFTER_SPLIT" are only meaningful under LegacyPM. @aeubanks , hi
what's the status of LegacyPM? Do we need to care about the correctness for the
LegacyPM? Or we could give up on it?
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
