[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

[Chuanqi Xu via Phabricator via cfe-commits]

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