Page MenuHomePhabricator

[mlir] Update LLVMIR Fastmath flags use of MLIR BitEnum functionality
ClosedPublic

Authored by jfurtek on Apr 30 2022, 2:11 PM.

Details

Summary

This diff updates the LLVMIR dialect Fastmath flags attribute to use recently
added features of BitEnum attributes. Specifically, this diff uses the bit
enum "group" case to represent the fast value as an alias for a combination
of other values (ninf, nnan, ...), instead of using a separate integer
value. (This is in line with LLVM's fastmath flags representation.) This diff
also leverages the printBitEnumPrimaryGroups tblgen field for concise
enum printing.

The BitEnum features were developed for an upcoming diff that adds fastmath
support to the arithmetic dialect. This diff simply applies some of the relevant
new features to the LLVM dialect attribute.

Diff Detail

Event Timeline

jfurtek created this revision.Apr 30 2022, 2:11 PM
Herald added a project: Restricted Project. · View Herald Transcript
jfurtek requested review of this revision.Apr 30 2022, 2:11 PM
ftynse accepted this revision.May 2 2022, 1:04 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
39

Nit: can this be ", " (with the trailing space) so we still have it in the generated IR?

This revision is now accepted and ready to land.May 2 2022, 1:04 AM
jfurtek added inline comments.May 2 2022, 10:03 AM
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
39

I think that there is a small "can of worms" here...

  1. Adding desired spaces to the separator field works well for printing, but less so for parsing. The generated enum printing function (stringify()) uses llvm::join(), and that would work with ", ". However, unless modified, the generated parse function (symbolize()) would fail if a string without the space is encountered (e.g. enum1,enum2).
  2. Another alternative would be to automatically add a space after the separator in the tblgen-erated print() function. However, this is problematic because the other "common" enum separator is a vertical bar. So while enum1, enum2 looks OK, enum1| enum2 is less appealing.

It might work if we allow desired spacing in the tblgen string (separator), use that for printing, and then "strip" the separator value of whitespace for the generated parse() function. (I haven't tried, but it might work.)

I could also just check the separator string in mlir-tblgen and specialize whitespace printing for , and |, with a default for other separators.

Suggestions on the best approach are welcome...

jfurtek updated this revision to Diff 427672.May 6 2022, 10:24 AM
  • Update handling of separators with spaces for nicer printing.
jfurtek marked an inline comment as done.May 6 2022, 10:28 AM
jfurtek added inline comments.
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
39

The update addresses this - spaces in the ODS separator field are used for printing, but are stripped away for parsing (so that parsing accepts IR without spaces).

jfurtek updated this revision to Diff 427722.May 6 2022, 12:50 PM
  • Fix clang formatting
jfurtek updated this revision to Diff 428971.May 12 2022, 9:24 AM
  • Update handling of separators with spaces for nicer printing.
  • Fix clang formatting
  • rebase
Mogball accepted this revision.May 17 2022, 11:18 AM