In D151923#4389405, @mehdi_amini wrote:

I think this LG, but David had to update clang/lib/AST/MicrosoftMangle.cpp when some other float format was added, you don't need this here?

• Add newline to EOF
• Remove GPU from EmitC dependencies and remove GPU function attribute pass
• Revert small differences from original upstream

• Add newline to EOF

Addressed comments. Thank you @vzakhari .

• Fix cut-and-paste error in comment
• Rebase. Use tblgen-erated bit enum function. Remove unnecessary braces.

• Fix cut-and-paste error in comment

• Fix clang formatting
• Change fastmath print/parse format based on review comments. Refactor attribute lowering.
• Update MLIRArithmeticDialect CMake reference
• Add fastmath attribute binding to arith canonicalizations
• Fixes for compile failures after rebase
• Modify LLVM fastmath interface to be attribute-based instead of value-based.
• Update LLVM lowering logic to drop fastmath attributes for targets that do not support the interface.
• Remove arith dependency from LLVMCommon. Move attribute conversion to the specific conversion module (in this case ArithToLLVM).
• clang-format fix
• clang formatting fix
• More clang-formatting

I might be wrong, and I was just leaving a passing comment as I saw it

• Fix clang formatting
• Change fastmath print/parse format based on review comments. Refactor attribute lowering.
• Update MLIRArithmeticDialect CMake reference
• Add fastmath attribute binding to arith canonicalizations
• Fixes for compile failures after rebase
• Modify LLVM fastmath interface to be attribute-based instead of value-based.
• Update LLVM lowering logic to drop fastmath attributes for targets that do not support the interface.
• Remove arith dependency from LLVMCommon. Move attribute conversion to the specific conversion module (in this case ArithToLLVM).
• clang-formatting fix
• clang formatting fix

• Fix clang formatting
• Change fastmath print/parse format based on review comments. Refactor attribute lowering.
• Update MLIRArithmeticDialect CMake reference
• Add fastmath attribute binding to arith canonicalizations
• Fixes for compile failures after rebase
• Modify LLVM fastmath interface to be attribute-based instead of value-based.
• Update LLVM lowering logic to drop fastmath attributes for targets that do not support the interface.
• Remove arith dependency from LLVMCommon. Move attribute conversion to the specific conversion module (in this case ArithToLLVM).

It seems weird to add a dependency on the Arith Dialect here.

• Update LLVM lowering logic to drop fastmath attributes for targets that do not support the interface.

• Fix clang formatting
• Change fastmath print/parse format based on review comments. Refactor attribute lowering.
• Update MLIRArithmeticDialect CMake reference
• Add fastmath attribute binding to arith canonicalizations
• Fixes for compile failures after rebase
• Modify LLVM fastmath interface to be attribute-based instead of value-based.

Abandoning for now - creating a separate diff that removes the tblgen option bit for default-valued attributes.

• rebase

I don't think this bit should be on attributes at all (so -1 to 2),

...

+1 to operation format rather than attribute specific.

@rriddle @mehdi_amini Is there any chance we can come to a resolution on this? (I'd like to land the fastmath support - which has a default-valued attribute - and a couple of others have expressed interest in it.)

Haven't looked deeply but this kind of feels weird, why wouldn't we just always elide the default?

Adding @Mogball since this is based heavily on his diff for optional parameters (https://reviews.llvm.org/D133816)

Will you be able to land this in the next two weeks?

In D126305#3820641, @kiranchandramohan wrote:

Thanks for working on this @jfurtek. Will you be able to land this in the next two weeks?

Is there any chance someone with commit privileges can land this? It was approved a month ago.

Thank you for working on this

I don't have commit privileges - can someone land this when convenient? Thanks!

• Rebase

I think that this has the potential to improve Python bindings for operations with default-valued attributes. (I believe that at the moment, in most cases the generated Python op creation code requires a value to be provided for an attribute that has a default value.)

Abandoning - these (trivial) changes have been folded in to the diff below, making this diff redundant.

• Modify AttrOrTypeParameter verifier codegen to return LogicalResult
• Use early return as per review
• Add mlir-tblgen-eration of verifyInvariants
• Generate multiple verifyInvariants() functions - one per builder
• Fix some build errors
• Add using declaration for verifyInvariants() and a default variadic template implementation
• Add generation of multiple verify() functions
• Update AttrOrTypeDef to use optional verifyParameters field. Add documentation and an additional unit test.
• Revert FloatAttr::verify() type change. Fix formatting.
• Fix formatting
• Remove unnecessary use of verifyParameters tblgen field
• Fix clang formatting
• Revert unintended function changes
• Update documentation and clean up control flow based on review feedback

• Fix clang formatting

• Modify AttrOrTypeParameter verifier codegen to return LogicalResult
• Use early return as per review
• Add mlir-tblgen-eration of verifyInvariants
• Generate multiple verifyInvariants() functions - one per builder
• Fix some build errors
• Add using declaration for verifyInvariants() and a default variadic template implementation
• Add generation of multiple verify() functions
• Update AttrOrTypeDef to use optional verifyParameters field. Add documentation and an additional unit test.
• Revert FloatAttr::verify() type change. Fix formatting.
• Fix formatting
• Remove unnecessary use of verifyParameters tblgen field

Adding @mehdi_amini to suggest reviewers, since none were automatically triggered.

Subsequent code for python bindings of default-valued attributes has broken the code in this diff - needs to be fixed.

• Modify AttrOrTypeParameter verifier codegen to return LogicalResult
• Use early return as per review
• Add mlir-tblgen-eration of verifyInvariants
• Generate multiple verifyInvariants() functions - one per builder
• Fix some build errors
• Add using declaration for verifyInvariants() and a default variadic template implementation
• Add generation of multiple verify() functions
• Update AttrOrTypeDef to use optional verifyParameters field. Add documentation and an additional unit test.
• Revert FloatAttr::verify() type change. Fix formatting.
• Fix formatting

• Remove redundant assert on attribute cast<>()

• Change fastmath print/parse format based on review comments.
• Refactor attribute lowering.

• rebase

This still needs work - I am just changing the state to "requesting review" to continue the discussion.

The current implementation uses hexadecimal, which requires 16 bits (2 bytes) to encode 8 bits of binary data. base64 would be an improvement over the current implementation that uses hexadecimal, requiring 8 bits to encode 6 bits of binary data. (The other constraint here is that encoding needs to be handled by the parser, which in the original example, means it can't have a quotation mark.) IMO, base64 is:

• smaller than hex
• somewhat ubiquitous
• compatible with the existing parser (I think?)
• a reasonable default for situations where the size overhead is an acceptable tradeoff for having the MLIR file be text-based and self-contained.

Have you considered using base64 instead of hex for binary data?

• Use early return as per review

• Modify AttrOrTypeParameter verifier codegen to return LogicalResult

Adding @mehdi_amini since he had some feedback on a previous iteration of this functionality.

OK - I think I understand. Let me take another pass at this. Thanks for the help.

• Fix clang formatting

In D126108#3533577, @rriddle wrote:

Did you consider adding verification support to AttrOrTypeParameter instead? That would be more inline with how verification is done for operations.

@stellaraccident - I don't have commit privileges. Can you (or someone else) land this diff at your convenience? Thanks!

• Update _ml_program_ops_ext.py to use FuncOp keyword arg

• Fix clang-format errors

(rebase)

• Update handling of separators with spaces for nicer printing.
• Fix clang formatting
• rebase

• Fix clang formatting

• Update handling of separators with spaces for nicer printing.

• Rebase

• Rebase

• Rebase

• Correct formatting, add comments, use llvm::reverse()
• Make formatting code more readable
• Properly punctuate comments.

• Improve code flow and rebase

• Correct formatting, add comments, use llvm::reverse()

• Update lexer to support vertical bar token, and add test for vertical bar case