(DRAFT) [mlir][ods] Don't allow trailing optional operands in assembly formats

[Mogball]

Summary

An optional operand at the end of an assembly format creates a format ambiguity with the SSA names of the results of the next operation, e.g.

// test.foo
$optional attr-dict

test.foo
%0 = test.bar

Prevent this by adding a verifier to check for these cases.

[Mogball]

make it compile at least

[lattner]

Cool, great catch!

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

613

This should be ok: it isn't a trailing argument, it has a trailing type.

mlir/include/mlir/Dialect/PDL/IR/PDLOps.td

475

Does the parser correctly disambiguate whether it is parsing replValues or replOperation here? That requires it to look for a colon after .. with (%4 to decode which path it is in

[Mogball]

Basically, I threw this patch up because I don't think, as things stand, there's a great way to detect this ambiguity. A lot of ops use the format $variadicOperands attr-dict require at least 1 operand or have some other invariant that make the format not actually ambiguous. Enforcing this check would make a lot of existing formats uglier than they need to be...

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

613

Right, but the guard on the optional group is parseOptionalOperand, so if it's empty, the parser will read into the results of the next op.

It's the same situation as you reported here https://github.com/llvm/llvm-project/issues/44860 (why I tagged you)

mlir/include/mlir/Dialect/PDL/IR/PDLOps.td

475

Uh, it won't. Good catch.

In reality, however, this operation will have either replValues or replOperation, so the format as it was is not ambiguous, but there's no way to communicate this to ODS.

[lattner]

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

613

Ah right, got it!

mlir/include/mlir/Dialect/PDL/IR/PDLOps.td

475

Right, I was wondering if ODS would catch the ambiguity. :-)