[mlir] Added oilist primitive

[shraiysh]

Summary

This patch attempts to add the oilist primitive proposed in the RFC: Extending Declarative Assembly Format to support order-independent variadic segments.

This element supports optional order-independent variadic segments for operations. This will allow OpenACC and OpenMP Dialects to have similar and relaxed requirements while encouraging the use of Declarative Assembly Format and avoiding code duplication.

An oilist element parses grammar of the form:

clause-list := clause clause-list | empty
clause := `keyword` clause1 | `otherKeyword` clause2
clause1 := <assembly-format element>
clause2 := <assembly-format element>

AssemblyFormat specification:

let assemblyFormat = [{
  oilist( `keyword` clause1
        | `otherkeyword` clause2
        ...
        )
}];

Example:

oilist( `private` `(` $arg0 `:` type($arg0) `)`
      | `nowait`
      | `reduction` custom<ReductionClause>($arg1, type($arg1)))

oilist( `private` `=` $arg0 `:` type($arg0)
      | `reduction` `=` $arg1 `:` type($arg1)
      | `firstprivate` `=` $arg3 `:` type($arg2))

[shraiysh]

Remove unrelated change

[Mogball]

Nice!

mlir/test/mlir-tblgen/op-format-spec.td

368

Can you add an op to the test dialect that uses this new directive and then add some list tests to test round-tripping the new format?

mlir/tools/mlir-tblgen/FormatGen.h

49

[ and ] are called l_square and r_square in other parts of the codebase.

mlir/tools/mlir-tblgen/OpFormatGen.cpp

46

Because oilist is a keyword, I would consider it more of a "directive" and wouldn't really group it with optional.

256–257

263

Can you encapsulate these members and document them?

691

Can you be more descriptive here?

693–696

Can you also reference oilist somewhere in the error message? Otherwise, it might be a bit vague to someone who isn't aware of the construct.

792–796

1187

Use cast if you're immediately going to dereference.

1188–1189

If you're generating an int for every element whose only purpose is to index into a BitVector, why not just generate a bool flag for each element instead and ditch the bit vector?

1193

You could instead do

::llvm::StringRef oilistKey;
while (succeeded(parser.parseOptionalKeyword(&oilistKey))) {
  if (oilistKey == "literal0") { ... } // or StringSwitch
}

1198

1947

Handling the spaces can be tricky. I think you should delete this line and call genLiteralPrinter below (instead of directly generating the literal print yourself), which has the correct behaviour for printing spaces and updating the flags.

1952

1952–1953

hasAttrOfType?

1955–1960

Let genElementPrinter handle the spaces.

1964–1965

2704–2705

Can you add a comment explaining what this is meant to guard against? (I like the error message btw)

2851–2852

2855–2859

Leftover debugging?

2863

2871–2873

2875–2876

A ] is required to exit the loop, so this branch is unreachable.

[shraiysh]

Thanks for the detailed review @Mogball. I have tried to address all your comments. Please let me know if I missed something!

mlir/tools/mlir-tblgen/OpFormatGen.cpp

1193

I tried doing that earlier, but that was causing issues when this was the last element in the assembly format.

For example,
Operation Definition:

let assemblyFormat = [{ oilist [ `keyword` | `otherKeyword` ] }];

MLIR file:

func @testing() {
  test.op keyword
  return
}

It tends to accept return as a keyword and enters the while loop. Once it is parsed, I do not know how to un-parse it for the next operation to take over. Please let me know if you have an alternative solution for this.

[shraiysh]

mlir/tools/mlir-tblgen/OpFormatGen.cpp

263

I have documented them. I am not too sure if I can encapsulate literalElements as OIListElement is itself encapsulating it, but I guess we can do something about parsingElements. Please let me know if you want me to make a new class ElementListElement for one parsing element - it is a element that is a list of elements (along with its own parsing and printing). I know that the name is a bit weird, looking for better suggestions for the same.

2871–2873

I cannot put it in the while condition, because then this testcase will be accepted with no errors:

oilist [ `something` | ]

This is because after getting a pipe, continue will check the condition and it will break the loop as curToken is now ]. So, now this will get accepted without any error reporting.

[rriddle]

Thanks, will take a look within the next day or two.

mlir/test/lib/Dialect/Test/TestDialect.cpp

521–523

Drop trivial braces.

[Mogball]

mlir/tools/mlir-tblgen/OpFormatGen.cpp

263

I meant to make them private members and provide accessors:

auto getLiteralElements() const {
  return llvm::make_pointee_range(llvm::map_range(literalElements, [](auto &el) { return cast<LiteralElement>(el.get()); }));
}

695

1184–1187

Use a for-each loop (and the provided accessor)

1193

Ah, right, because there's no delimiter to end the list. This is fine as-is then.

1193

You can use llvm::zip to iterate over literalElements and parsingElements at the same time

for (auto it : llvm::zip(oilist->getLiteralElements(), oilist->getParsingElements())) {
  LiteralElement &literalElement = std::get<0>(it);
  auto &parsingElement = std::get<1>(it);
  ...
}

You can move printing else to the end of the loop without checking the index. Then after the loop print

body << " {\n"

2871–2873

Right, makes sense.

[rriddle]

Can you add documentation for this in docs/OpDefinitions.md?

Also, how does this interact with the detection of parse ambiguities? We should still detect ambiguities inside of (and adjacent to) oilist.

mlir/test/lib/Dialect/Test/TestOps.td

648–650

Why surround with brackets instead of using something like ()(like all of the other directives)?

mlir/tools/mlir-tblgen/FormatGen.cpp

177–179

Keep this sorted.

mlir/tools/mlir-tblgen/FormatGen.h

74–75

Keep this sorted.

mlir/tools/mlir-tblgen/OpFormatGen.cpp

45–46

Please keep these sorted.

255

691

typo: coressponding

695–696

Drop the reference to the current operation, leave that to the parser to handle infra to callout.

The message is also a bit confusing, this is calling out a duplicate entry for the clause but the message seems to state that only one clause of the list is valid.

1182–1183

1184

Cache the end iterator unless it changes within the loop

2705–2711

Can you flip the if so that error happens first? We generally prefer early exit.

2846

[shraiysh]

Thanks for the review @Mogball and @rriddle. I have tried to address all your comments.

@rriddle can you please elaborate on the parse ambiguities. Are you talking about something like this?

oilist ( `keyword` | `otherkeyword` ) `keyword` attr-dict

I realise this translates to no valid syntax with the current implementation of the parser. Would you suggest not allowing oilist-literals right after an oilist element? (Looking for ideas on this)

If there are any other ambiguities which should be discussed, we can discuss them before moving ahead.

[Mogball]

LG enough for me once the issues I've commented on below are resolved. You should wait for river to sign off too.

mlir/tools/mlir-tblgen/OpFormatGen.cpp

250

Constructors and public methods should come before private methods and class members

279–281

make_pointee_range will work on any range of types that implement operator*, including unique_ptr. You don't need to map them to get(). Also, I would just inline this map function into your accessor method below.

298–299

You don't need to store this in a std::function

1192

Should this be a reference?

1951

Should this be a reference?

[shraiysh]

Thanks for the review @Mogball. Addressed Comments.

mlir/tools/mlir-tblgen/OpFormatGen.cpp

250

I wanted the vectorConverter method to be private, but because its return type is auto, I was forced to put it before its use in getParsingElements(). So, if I try to put the other public methods above, the structure will be something like this. Please let me know if I should change it to this:

public:
  OIListElement(...) { ... }
  static bool classof(...) { ... }
  auto getLiteralElements() const {...}
private:
  static auto vectorConverter(...) { ... }
  std::vector<...> literalElements;
  std::vector<...> parsingElements;
public:
  auto getParsingElements() const {...}

279–281

Thanks for the unique_ptr comment. Addressed it.

About the inlining, I can make it an std::function there, but the declaration will not be pretty as it returns a very complex templated llvm::iterator_range. (can't use it directly or use auto with lambda, because lambdas cannot be copied and llvm::zip will try to copy lambda).

std::function<llvm::iterator_range<llvm::pointee_iterator<decltype((
    std::begin(std::declval<std::vector<std::unique_ptr<Element>>>())))>>(
    const std::vector<std::unique_ptr<Element>> &vec)>
    vectorConverter =
        [](auto &vec) { return llvm::make_pointee_range(vec); };

Let me know if you prefer this, and I will change it accordingly.

298–299

I cannot use auto as copy constructor for lambda functions is deleted but it is not deleted for std::function. The function is stored as is in a llvm::map_range iterator and it is copied while using llvm::zip. Using auto will make it a lambda and lead to errors.

[rriddle]

In D115215#3190124, @shraiysh wrote:

Thanks for the review @Mogball and @rriddle. I have tried to address all your comments.

@rriddle can you please elaborate on the parse ambiguities. Are you talking about something like this?

oilist ( `keyword` | `otherkeyword` ) `keyword` attr-dict

I realise this translates to no valid syntax with the current implementation of the parser. Would you suggest not allowing oilist-literals right after an oilist element? (Looking for ideas on this)

If there are any other ambiguities which should be discussed, we can discuss them before moving ahead.

One case in particular is https://github.com/llvm/llvm-project/blob/1652871473a70e9908d794e858388edc89cd6e88/mlir/tools/mlir-tblgen/OpFormatGen.cpp#L2435
https://github.com/llvm/llvm-project/blob/1652871473a70e9908d794e858388edc89cd6e88/mlir/test/mlir-tblgen/op-format-spec.td#L463

mlir/docs/OpDefinitions.md

625

I would avoid the use of "string" here.

627

Something non-obvious here is the treatment of non-optional constructs, should those be allowed? What happens in those cases? Should we error out?

mlir/tools/mlir-tblgen/OpFormatGen.cpp

250

That method doesn't look like it pays for itself, I'd just inline it and keep proper public -> private ordering.

253

Documentation should use /// comments.

274–280

This doesn't appear super useful, I'd just inline it wherever this gets called.

293–294

294

298–299

The use of std::function is quite unfortunate, can you add a comment here capturing this?

303–304

1192

Can you spell out auto here?

1196

nit: Make sure to use camelCase for variable names.

1199

nit: Spell out auto here.

[shraiysh]

Thanks for the review @rriddle. I have tried to address all the comments. Please let me know if I have missed something.

[shraiysh]

Ping!

[rriddle]

Thanks for the ping, appreciated!

mlir/tools/mlir-tblgen/OpFormatGen.cpp

2183

I think at head we can drop the llvm:: on these.

2522

nit: Drop the trivial braces here.

2887

Can we drop the ::mlir:: on these? (I'm not sure why they are on the others in this file).

[clementval]

mlir/docs/OpDefinitions.md

626

[shraiysh]

Thanks for accepting this @rriddle, and thanks for the review @clementval. Addressed comments.

@Mogball, I have rebased this patch with main and there were some substantial changes by you in these files. Can you please recheck if everything is where it is supposed to be in this patch. Thank you.

[Mogball]

Overall LGTM.

mlir/tools/mlir-tblgen/FormatGen.h

379

Fill this in? :P

380

Let's move this into OpFormatGen.cpp. There is an expectation that elements defined in this file are available to both Attribute/Type and Operation formats.

385–386

391

399

avoid returning a copy

mlir/tools/mlir-tblgen/OpFormatGen.cpp

793

avoid copying the vector

1190

1951

2910

Unneeded?

2916

Same here

[shraiysh]

Rebase with main, addressed comments. Thanks for the review @Mogball.

mlir/tools/mlir-tblgen/FormatGen.h

391

Sadly, I have to use std::function here because lambdas can't be copied (copy constructor is required in llvm::zip). I have added a comment about this.

mlir/docs/OpDefinitions.md

 
 *   `functional-type` ( inputs , results )
 
     -   Formats the `inputs` and `results` arguments as a
         [function type](Dialects/Builtin.md/#functiontype).
     -   The constraints on `inputs` and `results` are the same as the `input` of
         the `type` directive.
 
+*   `oilist` ( \`keyword\` elements | \`otherKeyword\` elements ...)
+
+    -   Represents an optional order-independent list of clauses. Each clause
+        has a keyword and corresponding assembly format.
+    -   Each clause can appear 0 or 1 time (in any order).
+    -   Only literals, types and variables can be used within an oilist element.
+    -   All the variables must be optional or variadic.
+
 *   `operands`
 
     -   Represents all of the operands of an operation.
 
 *   `ref` ( input )
 
     -   Represents a reference to the a variable or directive, that must have
         already been resolved, that may be used as a parameter to a `custom`

mlir/test/IR/traits.mlir

 func @succeededResultSizeAttr() {
   // CHECK: test.attr_sized_results
   %0:4 = "test.attr_sized_results"() {result_segment_sizes = dense<[0, 2, 1, 1]>: vector<4xi32>} : () -> (i32, i32, i32, i32)
   return
 }
 
 // -----
 
+// CHECK-LABEL: @succeededOilistTrivial
+func @succeededOilistTrivial() {
+  // CHECK: test.oilist_with_keywords_only keyword
+  test.oilist_with_keywords_only keyword
+  // CHECK: test.oilist_with_keywords_only otherKeyword
+  test.oilist_with_keywords_only otherKeyword
+  // CHECK: test.oilist_with_keywords_only keyword otherKeyword
+  test.oilist_with_keywords_only keyword otherKeyword
+  // CHECK: test.oilist_with_keywords_only keyword otherKeyword
+  test.oilist_with_keywords_only otherKeyword keyword
+  return
+}
+
+// -----
+
+// CHECK-LABEL: @succeededOilistSimple
+func @succeededOilistSimple(%arg0 : i32, %arg1 : i32, %arg2 : i32) {
+  // CHECK: test.oilist_with_simple_args keyword %{{.*}} : i32
+  test.oilist_with_simple_args keyword %arg0 : i32
+  // CHECK: test.oilist_with_simple_args otherKeyword %{{.*}} : i32
+  test.oilist_with_simple_args otherKeyword %arg0 : i32
+  // CHECK: test.oilist_with_simple_args thirdKeyword %{{.*}} : i32
+  test.oilist_with_simple_args thirdKeyword %arg0 : i32
+
+  // CHECK: test.oilist_with_simple_args keyword %{{.*}} : i32 otherKeyword %{{.*}} : i32
+  test.oilist_with_simple_args keyword %arg0 : i32 otherKeyword %arg1 : i32
+  // CHECK: test.oilist_with_simple_args keyword %{{.*}} : i32 thirdKeyword %{{.*}} : i32
+  test.oilist_with_simple_args keyword %arg0 : i32 thirdKeyword %arg1 : i32
+  // CHECK: test.oilist_with_simple_args otherKeyword %{{.*}} : i32 thirdKeyword %{{.*}} : i32
+  test.oilist_with_simple_args thirdKeyword %arg0 : i32 otherKeyword %arg1 : i32
+
+  // CHECK: test.oilist_with_simple_args keyword %{{.*}} : i32 otherKeyword %{{.*}} : i32 thirdKeyword %{{.*}} : i32
+  test.oilist_with_simple_args keyword %arg0 : i32 otherKeyword %arg1 : i32 thirdKeyword %arg2 : i32
+  // CHECK: test.oilist_with_simple_args keyword %{{.*}} : i32 otherKeyword %{{.*}} : i32 thirdKeyword %{{.*}} : i32
+  test.oilist_with_simple_args otherKeyword %arg0 : i32 keyword %arg1 : i32 thirdKeyword %arg2 : i32
+  // CHECK: test.oilist_with_simple_args keyword %{{.*}} : i32 otherKeyword %{{.*}} : i32 thirdKeyword %{{.*}} : i32
+  test.oilist_with_simple_args otherKeyword %arg0 : i32 thirdKeyword %arg1 : i32 keyword %arg2 : i32
+  return
+}
+
+// -----
+
+// CHECK-LABEL: @succeededOilistVariadic
+// CHECK-SAME: (%[[ARG0:.*]]: i32, %[[ARG1:.*]]: i32, %[[ARG2:.*]]: i32)
+func @succeededOilistVariadic(%arg0: i32, %arg1: i32, %arg2: i32) {
+  // CHECK: test.oilist_variadic_with_parens keyword(%[[ARG0]], %[[ARG1]] : i32, i32)
+  test.oilist_variadic_with_parens keyword (%arg0, %arg1 : i32, i32)
+  // CHECK: test.oilist_variadic_with_parens keyword(%[[ARG0]], %[[ARG1]] : i32, i32) otherKeyword(%[[ARG2]], %[[ARG1]] : i32, i32)
+  test.oilist_variadic_with_parens otherKeyword (%arg2, %arg1 : i32, i32) keyword (%arg0, %arg1 : i32, i32)
+  // CHECK: test.oilist_variadic_with_parens keyword(%[[ARG0]], %[[ARG1]] : i32, i32) otherKeyword(%[[ARG0]], %[[ARG1]] : i32, i32) thirdKeyword(%[[ARG2]], %[[ARG0]], %[[ARG1]] : i32, i32, i32)
+  test.oilist_variadic_with_parens thirdKeyword (%arg2, %arg0, %arg1 : i32, i32, i32) keyword (%arg0, %arg1 : i32, i32) otherKeyword (%arg0, %arg1 : i32, i32)
+  return
+}
+
+// -----
+// CHECK-LABEL: succeededOilistCustom
+// CHECK-SAME: (%[[ARG0:.*]]: i32, %[[ARG1:.*]]: i32, %[[ARG2:.*]]: i32)
+func @succeededOilistCustom(%arg0: i32, %arg1: i32, %arg2: i32) {
+  // CHECK: test.oilist_custom private(%[[ARG0]], %[[ARG1]] : i32, i32)
+  test.oilist_custom private (%arg0, %arg1 : i32, i32)
+  // CHECK: test.oilist_custom private(%[[ARG0]], %[[ARG1]] : i32, i32) nowait
+  test.oilist_custom private (%arg0, %arg1 : i32, i32) nowait
+  // CHECK: test.oilist_custom private(%arg0, %arg1 : i32, i32) nowait reduction (%arg1)
+  test.oilist_custom nowait reduction (%arg1) private (%arg0, %arg1 : i32, i32)
+  return
+}
+
+// -----
+
 func @failedHasDominanceScopeOutsideDominanceFreeScope() -> () {
   "test.ssacfg_region"() ({
     test.graph_region {
       // expected-error @+1 {{operand #0 does not dominate this use}}
       %2:3 = "bar"(%1) : (i64) -> (i1,i1,i1)
     }
     // expected-note @+1 {{operand defined here}}
     %1 = "baz"() : () -> (i64)

 // -----
 
 // Check that we can query traits in attributes
 func @failed_attr_traits() {
   // expected-error@+1 {{'attr' attribute should have trait 'TestAttrTrait'}}
   "test.attr_with_trait"() {attr = 42 : i32} : () -> ()
   return
 }
 No newline at end of file

mlir/test/lib/Dialect/Test/TestDialect.cpp

 
 //===----------------------------------------------------------------------===//
 // Test Format* operations
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // Parsing
 
+static ParseResult
+parseCustomOptionalOperand(OpAsmParser &parser,
+                           Optional<OpAsmParser::OperandType> &optOperand) {
+  if (succeeded(parser.parseOptionalLParen())) {
+    optOperand.emplace();
+    if (parser.parseOperand(*optOperand) || parser.parseRParen())
+      return failure();
+  }
+  return success();
+}
+
 static ParseResult parseCustomDirectiveOperands(
     OpAsmParser &parser, OpAsmParser::OperandType &operand,
     Optional<OpAsmParser::OperandType> &optOperand,
     SmallVectorImpl<OpAsmParser::OperandType> &varOperands) {
   if (parser.parseOperand(operand))
     return failure();
   if (succeeded(parser.parseOptionalComma())) {
     optOperand.emplace();
if (parser.parseInteger(operandCount))
     return failure();
   bool expectedOptionalOperand = operandCount == 0;
   return success(expectedOptionalOperand != optOperand.hasValue());
 }
 
 //===----------------------------------------------------------------------===//
 // Printing
 
+static void printCustomOptionalOperand(OpAsmPrinter &printer, Operation *,
+                                       Value optOperand) {
+  if (optOperand)
+    printer << "(" << optOperand << ") ";
+}
+
 static void printCustomDirectiveOperands(OpAsmPrinter &printer, Operation *,
                                          Value operand, Value optOperand,
                                          OperandRange varOperands) {
   printer << operand;
   if (optOperand)
     printer << ", " << optOperand;
   printer << " -> (" << varOperands << ")";
 }

mlir/test/lib/Dialect/Test/TestOps.td

let results = (outs
     Variadic<I32>:$d
   );
 }
 
 // This is used to test that the fallback for a custom op's parser and printer
 // is the dialect parser and printer hooks.
 def CustomFormatFallbackOp : TEST_Op<"dialect_custom_format_fallback">;
 
+// Ops related to OIList primitive
+def OIListTrivial : TEST_Op<"oilist_with_keywords_only"> {
+  let assemblyFormat = [{
+    oilist( `keyword`
+          | `otherKeyword`) attr-dict
+  }];
+}
+
+def OIListSimple : TEST_Op<"oilist_with_simple_args", [AttrSizedOperandSegments]> {
+  let arguments = (ins Optional<AnyType>:$arg0,
+                       Optional<AnyType>:$arg1,
+                       Optional<AnyType>:$arg2);
+  let assemblyFormat = [{
+    oilist( `keyword` $arg0 `:` type($arg0)
+          | `otherKeyword` $arg1 `:` type($arg1)
+          | `thirdKeyword` $arg2 `:` type($arg2) ) attr-dict
+  }];
+}
+
+def OIListVariadic : TEST_Op<"oilist_variadic_with_parens", [AttrSizedOperandSegments]> {
+  let arguments = (ins Variadic<AnyType>:$arg0,
+                       Variadic<AnyType>:$arg1,
+                       Variadic<AnyType>:$arg2);
+  let assemblyFormat = [{
+    oilist( `keyword` `(` $arg0 `:` type($arg0) `)`
+          | `otherKeyword` `(` $arg1 `:` type($arg1) `)`
+          | `thirdKeyword` `(` $arg2 `:` type($arg2) `)`) attr-dict
+  }];
+}
+
+def OIListCustom : TEST_Op<"oilist_custom", [AttrSizedOperandSegments]> {
+  let arguments = (ins Variadic<AnyType>:$arg0,
+                       Optional<I32>:$optOperand,
+                       UnitAttr:$nowait);
+  let assemblyFormat = [{
+    oilist( `private` `(` $arg0 `:` type($arg0) `)`
+          | `nowait`
+          | `reduction` custom<CustomOptionalOperand>($optOperand)
+    ) attr-dict
+  }];
+}
+
+def OIListAllowedLiteral : TEST_Op<"oilist_allowed_literal"> {
+  let assemblyFormat = [{
+    oilist( `foo` | `bar` ) `buzz` attr-dict
+  }];
+}
+
 // This is used to test encoding of a string attribute into an SSA name of a
 // pretty printed value name.
 def StringAttrPrettyNameOp
  : TEST_Op<"string_attr_pretty_name",
            [DeclareOpInterfaceMethods<OpAsmOpInterface, ["getAsmResultNames"]>]> {
   let arguments = (ins StrArrayAttr:$names);
   let results = (outs Variadic<I32>:$r);
   let hasCustomAssemblyFormat = 1;

mlir/test/mlir-tblgen/op-format-spec.td

 }]>;
 // CHECK-NOT: error
 def LiteralValid : TestFormat_Op<[{
   `_` `:` `,` `=` `<` `>` `(` `)` `[` `]` `?` `+` `*` ` ` `` `->` `\n` `abc$._`
   attr-dict
 }]>;
 
 //===----------------------------------------------------------------------===//
+// OIList Element
+//===----------------------------------------------------------------------===//
+
+// CHECK: error: format ambiguity because bar is used in two adjacent oilist elements.
+def OIListAdjacentOIList : TestFormat_Op<[{
+  oilist ( `foo` | `bar` ) oilist ( `bar` | `buzz` ) attr-dict
+}]>;
+// CHECK: error: expected literal, but got ')'
+def OIListErrorExpectedLiteral : TestFormat_Op<[{
+  oilist( `keyword` | ) attr-dict
+}]>;
+// CHECK: error: expected literal, but got ')'
+def OIListErrorExpectedEmpty : TestFormat_Op<[{
+  oilist() attr-dict
+}]>;
+// CHECK: error: expected literal, but got '$arg0'
+def OIListErrorNoLiteral : TestFormat_Op<[{
+  oilist( $arg0 `:` type($arg0) | $arg1 `:` type($arg1) ) attr-dict
+}], [AttrSizedOperandSegments]>, Arguments<(ins Optional<AnyType>:$arg0, Optional<AnyType>:$arg1)>;
+// CHECK: error: format ambiguity because foo is used both in oilist element and the adjacent literal.
+def OIListLiteralAmbiguity : TestFormat_Op<[{
+  oilist( `foo` | `bar` ) `foo` attr-dict
+}]>;
+// CHECK: error: expected '(' before oilist argument list
+def OIListStartingToken : TestFormat_Op<[{
+  oilist `wrong` attr-dict
+}]>;
+
+// CHECK-NOT: error
+def OIListTrivial : TestFormat_Op<[{
+  oilist(`keyword` `(` `)` | `otherkeyword` `(` `)`) attr-dict
+}]>;
+def OIListSimple : TestFormat_Op<[{
+  oilist( `keyword` $arg0 `:` type($arg0)
+        | `otherkeyword` $arg1 `:` type($arg1)
+        | `thirdkeyword` $arg2 `:` type($arg2) )
+  attr-dict
+}], [AttrSizedOperandSegments]>, Arguments<(ins Optional<AnyType>:$arg0, Optional<AnyType>:$arg1, Optional<AnyType>:$arg2)>;
+def OIListVariadic : TestFormat_Op<[{
+  oilist( `keyword` `(` $args0 `:` type($args0) `)`
+        | `otherkeyword` `(` $args1 `:` type($args1) `)`
+        | `thirdkeyword` `(` $args2 `:` type($args2) `)`)
+  attr-dict
+}], [AttrSizedOperandSegments]>, Arguments<(ins Variadic<AnyType>:$args0, Variadic<AnyType>:$args1, Variadic<AnyType>:$args2)>;
+def OIListCustom : TestFormat_Op<[{
+  oilist( `private` `(` $arg0 `:` type($arg0) `)`
+        | `nowait`
+        | `reduction` custom<ReductionClause>($arg1, type($arg1))) attr-dict
+}], [AttrSizedOperandSegments]>, Arguments<(ins Optional<AnyType>:$arg0, Optional<AnyType>:$arg1)>;
+
+//===----------------------------------------------------------------------===//
 // Optional Groups
 //===----------------------------------------------------------------------===//
 
 // CHECK: error: optional groups can only be used as top-level elements
 def OptionalInvalidA : TestFormat_Op<[{
   type(($attr^)?) attr-dict
 }]>, Arguments<(ins OptionalAttr<I64Attr>:$attr)>;
 // CHECK: error: expected literal, variable, directive, or optional group

mlir/tools/mlir-tblgen/FormatGen.h

public:
   enum Kind {
     // Markers.
     eof,
     error,
 
     // Tokens with no info.
     l_paren,
     r_paren,
     caret,
     colon,
     comma,
     equal,
     less,
     greater,
     question,
     star,
+    pipe,
 
     // Keywords.
     keyword_start,
     kw_attr_dict,
     kw_attr_dict_w_keyword,
     kw_custom,
     kw_functional_type,
+    kw_oilist,
     kw_operands,
     kw_params,
     kw_qualified,
     kw_ref,
     kw_regions,
     kw_results,
     kw_struct,
     kw_successors,
     kw_type,
     keyword_end,
 
     // String valued tokens.
     identifier,
     literal,
     variable,
   };
 
   FormatToken(Kind kind, StringRef spelling) : kind(kind), spelling(spelling) {}

 
 class DirectiveElement : public FormatElementBase<FormatElement::Directive> {
 public:
   /// These are the kinds of directives.
   enum Kind {
     AttrDict,
     Custom,
     FunctionalType,
+    OIList,
     Operands,
     Ref,
     Regions,
     Results,
     Successors,
     Type,
     Params,
     Struct
private:
   /// The index of the anchor element of the optional group within
   /// `thenElements`.
   unsigned anchorIndex;
   /// The index of the first element that is parsed in `thenElements`. That is,
   /// the first non-whitespace element.
   unsigned parseStart;
 };
 
 //===----------------------------------------------------------------------===//
 // FormatParserBase
 //===----------------------------------------------------------------------===//
 
 /// Base class for a parser that implements an assembly format. This class
 /// defines a common assembly format syntax and the creation of format elements.
 /// Subclasses will need to implement parsing for the format elements they
 /// support.
 class FormatParser {
 public:
   /// Vtable anchor.
   virtual ~FormatParser();
 
   /// Parse the assembly format.
   FailureOr<std::vector<FormatElement *>> parse();
 
 protected:
   /// The current context of the parser when parsing an element.
   enum Context {
     /// The element is being parsed in a "top-level" context, i.e. at the top of
     /// the format or in an optional group.
     TopLevelContext,
     /// The element is being parsed as a custom directive child.
     CustomDirectiveContext,
     /// The element is being parsed as a type directive child.
     TypeDirectiveContext,
     /// The element is being parsed as a reference directive child.
     RefDirectiveContext,
     /// The element is being parsed as a struct directive child.

mlir/tools/mlir-tblgen/FormatGen.cpp

FormatToken FormatLexer::lexToken() {
   case '?':
     return formToken(FormatToken::question, tokStart);
   case '(':
     return formToken(FormatToken::l_paren, tokStart);
   case ')':
     return formToken(FormatToken::r_paren, tokStart);
   case '*':
     return formToken(FormatToken::star, tokStart);
+  case '|':
+    return formToken(FormatToken::pipe, tokStart);
 
   // Ignore whitespace characters.
   case 0:
   case ' ':
   case '\t':
   case '\n':
     return lexToken();
 
FormatToken FormatLexer::lexIdentifier(const char *tokStart) {
   // Check to see if this identifier is a keyword.
   StringRef str(tokStart, curPtr - tokStart);
   auto kind =
       StringSwitch<FormatToken::Kind>(str)
           .Case("attr-dict", FormatToken::kw_attr_dict)
           .Case("attr-dict-with-keyword", FormatToken::kw_attr_dict_w_keyword)
           .Case("custom", FormatToken::kw_custom)
           .Case("functional-type", FormatToken::kw_functional_type)
+          .Case("oilist", FormatToken::kw_oilist)
           .Case("operands", FormatToken::kw_operands)
           .Case("params", FormatToken::kw_params)
           .Case("ref", FormatToken::kw_ref)
           .Case("regions", FormatToken::kw_regions)
           .Case("results", FormatToken::kw_results)
           .Case("struct", FormatToken::kw_struct)
           .Case("successors", FormatToken::kw_successors)
           .Case("type", FormatToken::kw_type)
           .Case("qualified", FormatToken::kw_qualified)
           .Default(FormatToken::identifier);
   return FormatToken(kind, str);
 }
 
 //===----------------------------------------------------------------------===//
 // FormatParser
 //===----------------------------------------------------------------------===//
 
 FormatElement::~FormatElement() = default;
FailureOr<FormatElement *> FormatParser::parseLiteral(Context ctx) {
   consumeToken();
 
   if (ctx != TopLevelContext) {
     return emitError(
         loc,
         "literals may only be used in the top-level section of the format");
   }
   // Get the spelling without the surrounding backticks.
-  StringRef value = tok.getSpelling().drop_front().drop_back();
+  StringRef value = tok.getSpelling();
+  // Prevents things like `$arg0` or empty literals (when a literal is expected
+  // but not found) from getting segmentation faults.
+  if (value.size() < 2 || value[0] != '`' || value[value.size() - 1] != '`')
+    return emitError(tok.getLoc(), "expected literal, but got '" + value + "'");
+  value = value.drop_front().drop_back();
 
   // The parsed literal is a space element (`` or ` `) or a newline.
   if (value.empty() || value == " " || value == "\\n")
     return create<WhitespaceElement>(value);
 
   // Check that the parsed literal is valid.
   if (!isValidLiteral(value, [&](Twine msg) {
         (void)emitError(loc, "expected valid literal but got '" + value +

mlir/tools/mlir-tblgen/OpFormatGen.cpp

 // VariableElement
 
 namespace {
 /// This class represents an instance of an op variable element. A variable
 /// refers to something registered on the operation itself, e.g. an operand,
 /// result, attribute, region, or successor.
 template <typename VarT, VariableElement::Kind VariableKind>
 class OpVariableElement : public VariableElementBase<VariableKind> {
 public:
   using Base = OpVariableElement<VarT, VariableKind>;
 
   /// Create an op variable element with the variable value.
   OpVariableElement(const VarT *var) : var(var) {}
 
   /// Get the variable.
   const VarT *getVar() { return var; }
 
 protected:
public:
   }
 
 private:
   /// The argument that is used to format the directive.
   FormatElement *arg;
 
   bool shouldBeQualifiedFlag = false;
 };
+
+/// This class represents a group of order-independent optional clauses. Each
+/// clause starts with a literal element and has a coressponding parsing
+/// element. A parsing element is a continous sequence of format elements.
+/// Each clause can appear 0 or 1 time.
+class OIListElement : public DirectiveElementBase<DirectiveElement::OIList> {
+public:
+  OIListElement(std::vector<FormatElement *> &&literalElements,
+                std::vector<std::vector<FormatElement *>> &&parsingElements)
+      : literalElements(std::move(literalElements)),
+        parsingElements(std::move(parsingElements)) {}
+
+  /// Returns a range to iterate over the LiteralElements.
+  auto getLiteralElements() const {
+    // The use of std::function is unfortunate but necessary here. Lambda
+    // functions cannot be copied but std::function can be copied. This copy
+    // constructor is used in llvm::zip.
+    std::function<LiteralElement *(FormatElement * el)>
+        literalElementCastConverter =
+            [](FormatElement *el) { return cast<LiteralElement>(el); };
+    return llvm::map_range(literalElements, literalElementCastConverter);
+  }
+
+  /// Returns a range to iterate over the parsing elements corresponding to the
+  /// clauses.
+  ArrayRef<std::vector<FormatElement *>> getParsingElements() const {
+    return parsingElements;
+  }
+
+  /// Returns a range to iterate over tuples of parsing and literal elements.
+  auto getClauses() const {
+    return llvm::zip(getLiteralElements(), getParsingElements());
+  }
+
+private:
+  /// A vector of `LiteralElement` objects. Each element stores the keyword
+  /// for one case of oilist element. For example, an oilist element along with
+  /// the `literalElements` vector:
+  /// ```
+  ///  oilist [ `keyword` `=` `(` $arg0 `)` | `otherKeyword` `<` $arg1 `>`]
+  ///  literalElements = { `keyword`, `otherKeyword` }
+  /// ```
+  std::vector<FormatElement *> literalElements;
+
+  /// A vector of valid declarative assembly format vectors. Each object in
+  /// parsing elements is a vector of elements in assembly format syntax.
+  /// For example, an oilist element along with the parsingElements vector:
+  /// ```
+  ///  oilist [ `keyword` `=` `(` $arg0 `)` | `otherKeyword` `<` $arg1 `>`]
+  ///  parsingElements = {
+  ///    { `=`, `(`, $arg0, `)` },
+  ///    { `<`, $arg1, `>` }
+  ///  }
+  /// ```
+  std::vector<std::vector<FormatElement *>> parsingElements;
+};
 } // namespace
 
 //===----------------------------------------------------------------------===//
 // OperationFormat
 //===----------------------------------------------------------------------===//
 
 namespace {
 
 using ConstArgument =
     llvm::PointerUnion<const NamedAttribute *, const NamedTypeConstraint *>;
 
 struct OperationFormat {
   /// This class represents a specific resolver for an operand or result type.
   class TypeResolution {
   public:
     TypeResolution() = default;
 
     /// Get the index into the buildable types for this type, or None.
     Optional<int> getBuilderIdx() const { return builderIdx; }
     void setBuilderIdx(int idx) { builderIdx = idx; }
 
     /// Get the variable this type is resolved to, or nullptr.
     const NamedTypeConstraint *getVariable() const {
       return resolver.dyn_cast<const NamedTypeConstraint *>();
     }
     /// Get the attribute this type is resolved to, or nullptr.
     const NamedAttribute *getAttribute() const {
       return resolver.dyn_cast<const NamedAttribute *>();
     }
     /// Get the transformer for the type of the variable, or None.
     Optional<StringRef> getVarTransformer() const {
       return variableTransformer;
     }
     void setResolver(ConstArgument arg, Optional<StringRef> transformer) {
       resolver = arg;
       variableTransformer = transformer;
       assert(getVariable() || getAttribute());
     }
 
   private:
     /// If the type is resolved with a buildable type, this is the index into
     /// 'buildableTypes' in the parent format.
     Optional<int> builderIdx;
     /// If the type is resolved based upon another operand or result, this is
     /// the variable or the attribute that this type is resolved to.
     ConstArgument resolver;
     /// If the type is resolved based upon another operand or result, this is
     /// a transformer to apply to the variable when resolving.
     Optional<StringRef> variableTransformer;
   };
 
   /// The context in which an element is generated.
   enum class GenContext {
     /// The element is generated at the top-level or with the same behaviour.
     Normal,
     /// The element is generated inside an optional group.
     Optional
   };
 
   OperationFormat(const Operator &op)
       : allOperands(false), allOperandTypes(false), allResultTypes(false),
         infersResultTypes(false) {
     operandTypes.resize(op.getNumOperands(), TypeResolution());
     resultTypes.resize(op.getNumResults(), TypeResolution());
 
     hasImplicitTermTrait = llvm::any_of(op.getTraits(), [](const Trait &trait) {
       return trait.getDef().isSubClassOf("SingleBlockImplicitTerminator");
     });
 

 /// The code snippet used to generate a parser call for a successor.
 ///
 /// {0}: The name of the successor.
 const char *successorParserCode = R"(
   if (parser.parseSuccessor({0}Successor))
     return ::mlir::failure();
 )";
 
+/// The code snippet used to generate a parser for OIList
+///
+/// {0}: literal keyword corresponding to a case for oilist
+const char *oilistParserCode = R"(
+  if ({0}Clause) {
+    return parser.emitError(parser.getNameLoc())
+          << "`{0}` clause can appear at most once in the expansion of the "
+             "oilist directive";
+  }
+  {0}Clause = true;
+  result.addAttribute("{0}", UnitAttr::get(parser.getContext()));
+)";
+
 namespace {
 /// The type of length for a given parse argument.
 enum class ArgumentLengthKind {
   /// The argument is a variadic of a variadic, and may contain 0->N range
   /// elements.
   VariadicOfVariadic,
   /// The argument is variadic, and may contain 0->N elements.
   Variadic,
if (auto *optional = dyn_cast<OptionalElement>(element)) {
     if (anchor && anchor != elements.front() && anchor->isUnitAttr())
       elidedAnchorElement = anchor;
     for (FormatElement *childElement : elements)
       if (childElement != elidedAnchorElement)
         genElementParserStorage(childElement, op, body);
     for (FormatElement *childElement : optional->getElseElements())
       genElementParserStorage(childElement, op, body);
 
+  } else if (auto *oilist = dyn_cast<OIListElement>(element)) {
+    for (ArrayRef<FormatElement *> pelement : oilist->getParsingElements())
+      for (FormatElement *element : pelement)
+        genElementParserStorage(element, op, body);
+
   } else if (auto *custom = dyn_cast<CustomDirective>(element)) {
     for (FormatElement *paramElement : custom->getArguments())
       genElementParserStorage(paramElement, op, body);
 
   } else if (isa<OperandsDirective>(element)) {
     body << "  ::mlir::SmallVector<::mlir::OpAsmParser::OperandType, 4> "
             "allOperands;\n";
 
if (auto *optional = dyn_cast<OptionalElement>(element)) {
     if (!elseElements.empty()) {
       body << " else {\n";
       for (FormatElement *childElement : elseElements)
         genElementParser(childElement, body, attrTypeCtx);
       body << "  }";
     }
     body << "\n";
 
+    /// OIList Directive
+  } else if (OIListElement *oilist = dyn_cast<OIListElement>(element)) {
+    for (LiteralElement *le : oilist->getLiteralElements())
+      body << "  bool " << le->getSpelling() << "Clause = false;\n";
+
+    // Generate the parsing loop
+    body << "  while(true) {\n";
+    for (auto clause : oilist->getClauses()) {
+      LiteralElement *lelement = std::get<0>(clause);
+      ArrayRef<FormatElement *> pelement = std::get<1>(clause);
+      body << "if (succeeded(parser.parseOptional";
+      genLiteralParser(lelement->getSpelling(), body);
+      body << ")) {\n";
+      StringRef attrName = lelement->getSpelling();
+      body << formatv(oilistParserCode, attrName);
+      inferredAttributes.insert(attrName);
+      for (FormatElement *el : pelement)
+        genElementParser(el, body, attrTypeCtx);
+      body << "    } else ";
+    }
+    body << " {\n";
+    body << "    break;\n";
+    body << "  }\n";
+    body << "}\n";
+
     /// Literals.
   } else if (LiteralElement *literal = dyn_cast<LiteralElement>(element)) {
     body << "  if (parser.parse";
     genLiteralParser(literal->getSpelling(), body);
     body << ")\n    return ::mlir::failure();\n";
 
     /// Whitespaces.
   } else if (isa<WhitespaceElement>(element)) {
if (!elseElements.empty()) {
       }
       body << "  }";
     }
 
     body << "\n";
     return;
   }
 
+  // Emit the OIList
+  if (auto *oilist = dyn_cast<OIListElement>(element)) {
+    genLiteralPrinter(" ", body, shouldEmitSpace, lastWasPunctuation);
+    for (auto clause : oilist->getClauses()) {
+      LiteralElement *lelement = std::get<0>(clause);
+      ArrayRef<FormatElement *> pelement = std::get<1>(clause);
+
+      body << "  if ((*this)->hasAttrOfType<UnitAttr>(\""
+           << lelement->getSpelling() << "\")) {\n";
+      genLiteralPrinter(lelement->getSpelling(), body, shouldEmitSpace,
+                        lastWasPunctuation);
+      for (FormatElement *element : pelement) {
+        genElementPrinter(element, body, op, shouldEmitSpace,
+                          lastWasPunctuation);
+      }
+      body << "  }\n";
+    }
+    return;
+  }
+
   // Emit the attribute dictionary.
   if (auto *attrDict = dyn_cast<AttrDictDirective>(element)) {
     genAttrDictPrinter(*this, op, body, attrDict->isWithKeyword());
     lastWasPunctuation = false;
     return;
   }
 
   // Optionally insert a space before the next element. The AttrDict printer
private:
   /// Verify the state of operation results within the format.
   LogicalResult
   verifyResults(SMLoc loc,
                 llvm::StringMap<TypeResolutionInstance> &variableTyResolver);
 
   /// Verify the state of operation successors within the format.
   LogicalResult verifySuccessors(SMLoc loc);
 
+  LogicalResult verifyOIListElements(SMLoc loc,
+                                     ArrayRef<FormatElement *> elements);
+
   /// Given the values of an `AllTypesMatch` trait, check for inferable type
   /// resolution.
   void handleAllTypesMatchConstraint(
       ArrayRef<StringRef> values,
       llvm::StringMap<TypeResolutionInstance> &variableTyResolver);
   /// Check for inferable type resolution given all operands, and or results,
   /// have the same type. If 'includeResults' is true, the results also have the
   /// same type as all of the operands.
private:
   /// within the format.
   ConstArgument findSeenArg(StringRef name);
 
   /// Parse the various different directives.
   FailureOr<FormatElement *> parseAttrDictDirective(SMLoc loc, Context context,
                                                     bool withKeyword);
   FailureOr<FormatElement *> parseFunctionalTypeDirective(SMLoc loc,
                                                           Context context);
+  FailureOr<FormatElement *> parseOIListDirective(SMLoc loc, Context context);
+  LogicalResult verifyOIListParsingElement(FormatElement *element, SMLoc loc);
   FailureOr<FormatElement *> parseOperandsDirective(SMLoc loc, Context context);
   FailureOr<FormatElement *> parseQualifiedDirective(SMLoc loc,
                                                      Context context);
   FailureOr<FormatElement *> parseReferenceDirective(SMLoc loc,
                                                      Context context);
   FailureOr<FormatElement *> parseRegionsDirective(SMLoc loc, Context context);
   FailureOr<FormatElement *> parseResultsDirective(SMLoc loc, Context context);
   FailureOr<FormatElement *> parseSuccessorsDirective(SMLoc loc,
if (def.isSubClassOf("AllTypesMatch")) {
       }
     }
   }
 
   // Verify the state of the various operation components.
   if (failed(verifyAttributes(loc, elements)) ||
       failed(verifyResults(loc, variableTyResolver)) ||
       failed(verifyOperands(loc, variableTyResolver)) ||
-      failed(verifyRegions(loc)) || failed(verifySuccessors(loc)))
+      failed(verifyRegions(loc)) || failed(verifySuccessors(loc)) ||
+      failed(verifyOIListElements(loc, elements)))
     return failure();
 
   // Collect the set of used attributes in the format.
   fmt.usedAttributes = seenAttrs.takeVector();
   return success();
 }
 
 LogicalResult
if (!seenSuccessors.count(&successor)) {
                                   successor.name + "', not found",
                               "suggest adding a '$" + successor.name +
                                   "' directive to the custom assembly format");
     }
   }
   return success();
 }
 
+LogicalResult
+OpFormatParser::verifyOIListElements(SMLoc loc,
+                                     ArrayRef<FormatElement *> elements) {
+  // Check that all of the successors are within the format.
+  SmallVector<StringRef> prohibitedLiterals;
+  for (FormatElement *it : elements) {
+    if (auto *oilist = dyn_cast<OIListElement>(it)) {
+      if (!prohibitedLiterals.empty()) {
+        // We just saw an oilist element in last iteration. Literals should not
+        // match.
+        for (LiteralElement *literal : oilist->getLiteralElements()) {
+          if (find(prohibitedLiterals, literal->getSpelling()) !=
+              prohibitedLiterals.end()) {
+            return emitError(
+                loc, "format ambiguity because " + literal->getSpelling() +
+                         " is used in two adjacent oilist elements.");
+          }
+        }
+      }
+      for (LiteralElement *literal : oilist->getLiteralElements())
+        prohibitedLiterals.push_back(literal->getSpelling());
+    } else if (auto *literal = dyn_cast<LiteralElement>(it)) {
+      if (find(prohibitedLiterals, literal->getSpelling()) !=
+          prohibitedLiterals.end()) {
+        return emitError(
+            loc,
+            "format ambiguity because " + literal->getSpelling() +
+                " is used both in oilist element and the adjacent literal.");
+      }
+      prohibitedLiterals.clear();
+    } else {
+      prohibitedLiterals.clear();
+    }
+  }
+  return success();
+}
+
 void OpFormatParser::handleAllTypesMatchConstraint(
     ArrayRef<StringRef> values,
     llvm::StringMap<TypeResolutionInstance> &variableTyResolver) {
   for (unsigned i = 0, e = values.size(); i != e; ++i) {
     // Check to see if this value matches a resolved operand or result type.
     ConstArgument arg = findSeenArg(values[i]);
     if (!arg)
       continue;
OpFormatParser::parseDirectiveImpl(SMLoc loc, FormatToken::Kind kind,
   case FormatToken::kw_results:
     return parseResultsDirective(loc, ctx);
   case FormatToken::kw_successors:
     return parseSuccessorsDirective(loc, ctx);
   case FormatToken::kw_ref:
     return parseReferenceDirective(loc, ctx);
   case FormatToken::kw_type:
     return parseTypeDirective(loc, ctx);
+  case FormatToken::kw_oilist:
+    return parseOIListDirective(loc, ctx);
 
   default:
     return emitError(loc, "unsupported directive kind");
   }
 }
 
 FailureOr<FormatElement *>
 OpFormatParser::parseAttrDictDirective(SMLoc loc, Context context,
                                        bool withKeyword) {
   if (context == TypeDirectiveContext)
     return emitError(loc, "'attr-dict' directive can only be used as a "
                           "top-level directive");
 
   if (context == RefDirectiveContext) {
     if (!hasAttrDict)
       return emitError(loc, "'ref' of 'attr-dict' is not bound by a prior "
                             "'attr-dict' directive");
 
     // Otherwise, this is a top-level context.
   } else {
     if (hasAttrDict)
OpFormatParser::parseSuccessorsDirective(SMLoc loc, Context context) {
   } else {
     if (hasAllSuccessors || !seenSuccessors.empty())
       return emitError(loc, "'successors' directive creates overlap in format");
     hasAllSuccessors = true;
   }
   return create<SuccessorsDirective>();
 }
 
+FailureOr<FormatElement *>
+OpFormatParser::parseOIListDirective(SMLoc loc, Context context) {
+  if (failed(parseToken(FormatToken::l_paren,
+                        "expected '(' before oilist argument list")))
+    return failure();
+  std::vector<FormatElement *> literalElements;
+  std::vector<std::vector<FormatElement *>> parsingElements;
+  do {
+    FailureOr<FormatElement *> lelement = parseLiteral(context);
+    if (failed(lelement))
+      return failure();
+    literalElements.push_back(*lelement);
+    parsingElements.push_back(std::vector<FormatElement *>());
+    std::vector<FormatElement *> &currParsingElements = parsingElements.back();
+    while (peekToken().getKind() != FormatToken::pipe &&
+           peekToken().getKind() != FormatToken::r_paren) {
+      FailureOr<FormatElement *> pelement = parseElement(context);
+      if (failed(pelement) ||
+          failed(verifyOIListParsingElement(*pelement, loc)))
+        return failure();
+      currParsingElements.push_back(*pelement);
+    }
+    if (peekToken().getKind() == FormatToken::pipe) {
+      consumeToken();
+      continue;
+    }
+    if (peekToken().getKind() == FormatToken::r_paren) {
+      consumeToken();
+      break;
+    }
+  } while (true);
+
+  return create<OIListElement>(std::move(literalElements),
+                               std::move(parsingElements));
+}
+
+LogicalResult OpFormatParser::verifyOIListParsingElement(FormatElement *element,
+                                                         SMLoc loc) {
+  return TypeSwitch<FormatElement *, LogicalResult>(element)
+      // Only optional attributes can be within an oilist parsing group.
+      .Case([&](AttributeVariable *attrEle) {
+        if (!attrEle->getVar()->attr.isOptional())
+          return emitError(loc, "only optional attributes can be used to "
+                                "in an oilist parsing group");
+        return success();
+      })
+      // Only optional-like(i.e. variadic) operands can be within an oilist
+      // parsing group.
+      .Case([&](OperandVariable *ele) {
+        if (!ele->getVar()->isVariableLength())
+          return emitError(loc, "only variable length operands can be "
+                                "used within an oilist parsing group");
+        return success();
+      })
+      // Only optional-like(i.e. variadic) results can be within an oilist
+      // parsing group.
+      .Case([&](ResultVariable *ele) {
+        if (!ele->getVar()->isVariableLength())
+          return emitError(loc, "only variable length results can be "
+                                "used within an oilist parsing group");
+        return success();
+      })
+      .Case([&](RegionVariable *) {
+        // TODO: When ODS has proper support for marking "optional" regions, add
+        // a check here.
+        return success();
+      })
+      .Case([&](TypeDirective *ele) {
+        return verifyOIListParsingElement(ele->getArg(), loc);
+      })
+      .Case([&](FunctionalTypeDirective *ele) {
+        if (failed(verifyOIListParsingElement(ele->getInputs(), loc)))
+          return failure();
+        return verifyOIListParsingElement(ele->getResults(), loc);
+      })
+      // Literals, whitespace, and custom directives may be used.
+      .Case<LiteralElement, WhitespaceElement, CustomDirective,
+            FunctionalTypeDirective, OptionalElement>(
+          [&](FormatElement *) { return success(); })
+      .Default([&](FormatElement *) {
+        return emitError(loc, "only literals, types, and variables can be "
+                              "used within an oilist group");
+      });
+}
+
 FailureOr<FormatElement *> OpFormatParser::parseTypeDirective(SMLoc loc,
                                                               Context context) {
   if (context == TypeDirectiveContext)
     return emitError(loc, "'type' cannot be used as a child of another `type`");
 
   bool isRefChild = context == RefDirectiveContext;
   FailureOr<FormatElement *> operand;
   if (failed(parseToken(FormatToken::l_paren,