This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Replace StrEnumAttr -> EnumAttr in core dialects
ClosedPublic

Authored by Mogball on Jan 17 2022, 2:11 PM.

Diff Detail

Event Timeline

Mogball created this revision.Jan 17 2022, 2:11 PM
Mogball requested review of this revision.Jan 17 2022, 2:11 PM
mehdi_amini accepted this revision.Jan 17 2022, 2:14 PM

Nice! I like the removal of string comparison to use enums instead :)

This revision is now accepted and ready to land.Jan 17 2022, 2:14 PM

Indeed. Surprisingly, some dialects required very little work to change (StrEnumAttr -> EnumAttr and fix the BUILD file and that's it).

jpienaar added inline comments.
mlir/include/mlir/Dialect/GPU/GPUOps.td
40

Unrelated here, but we have namespace here and dialect specified below (which has a namespace), feels a bit weird

Mogball added inline comments.Jan 17 2022, 5:03 PM
mlir/include/mlir/Dialect/GPU/GPUOps.td
40

Yeah. That's because enums aren't tied to any particular dialect. I would like to redo enum gen entirely once all enums are moved over to AttrDef.

Mogball updated this revision to Diff 400690.Jan 17 2022, 6:49 PM

Fix flang and remove StrEnumAttr from OMP/ACC directives gen

Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2022, 6:49 PM
rriddle accepted this revision.Jan 18 2022, 12:17 AM

LGTM, nice cleanup!

mlir/include/mlir/Dialect/GPU/CMakeLists.txt
35

Is the m(docm) here an accident?

mlir/include/mlir/Dialect/GPU/GPUOps.td
680

Please wrap this at 80 characters.

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
556
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
189
Mogball marked 4 inline comments as done.Jan 18 2022, 8:48 AM

Thanks for the review

Mogball updated this revision to Diff 400868.Jan 18 2022, 9:13 AM

review comments

This revision was landed with ongoing or failed builds.Jan 18 2022, 9:15 AM
This revision was automatically updated to reflect the committed changes.