This is an archive of the discontinued LLVM Phabricator instance.

[Coroutines] Convert coroutine.presplit to enum attr
ClosedPublic

Authored by ChuanqiXu on Jun 9 2022, 8:49 PM.

Details

Summary

This is required by @nikic in https://reviews.llvm.org/D127383 and this fixes a FIXME too.

@rjmccall I'm not sure if swift have emitted "coroutine.presplit" in the frontend. If it haven't do so, it should be fine.

@ezhulenev I'm not sure if the changes in mlir part calls the right API. Although it passes the tests, I haven't run mlir actually.

Diff Detail

Event Timeline

ChuanqiXu created this revision.Jun 9 2022, 8:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2022, 8:49 PM
ChuanqiXu requested review of this revision.Jun 9 2022, 8:49 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptJun 9 2022, 8:49 PM

You probably want to add this to llvm::UpgradeAttributes as well, to maintain support for old bitcode.

You probably want to add this to llvm::UpgradeAttributes as well, to maintain support for old bitcode.

Oh, I feel like it is not so meaningful to keep backward compatibility. We've done some break changes before. Since the primary users of LLVM Coroutines are frontends, it shouldn't be hard for them to update.

ezhulenev accepted this revision.Jun 10 2022, 12:05 PM

Yes, that's the correct MLIR way of passing attributes to LLVM.

This revision is now accepted and ready to land.Jun 10 2022, 12:05 PM

@rjmccall Does this look good to you? I am afraid to break swift's codes.

rjmccall accepted this revision.Jun 13 2022, 9:34 AM

LGTM. Don't worry about Swift, we'll handle it.

This revision was landed with ongoing or failed builds.Jun 13 2022, 11:24 PM
This revision was automatically updated to reflect the committed changes.

It seems this change triggers a warning:

https://buildkite.com/llvm-project/upstream-bazel/builds/31190#018160e2-0b96-4254-8986-f039b7f2e0a1

llvm-project/llvm/lib/Transforms/Utils/CodeExtractor.cpp:901:15: error: enumeration value 'PresplitCoroutine' not handled in switch [-Werror,-Wswitch]

switch (Attr.getKindAsEnum()) {

It seems this change triggers a warning:

https://buildkite.com/llvm-project/upstream-bazel/builds/31190#018160e2-0b96-4254-8986-f039b7f2e0a1

llvm-project/llvm/lib/Transforms/Utils/CodeExtractor.cpp:901:15: error: enumeration value 'PresplitCoroutine' not handled in switch [-Werror,-Wswitch]

switch (Attr.getKindAsEnum()) {

Thanks for reporting! I am trying to fix it.