Let MLIR ODS also support generating build() functions without result type parameters when the op contains regions.

[phisiart]

Summary

Regions were intentionally left unsupported: https://github.com/llvm/llvm-project/commit/398f04aa49109fd5d1eff2c1946a2956dc6b29c6

[jpienaar]

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1586

This TODO refers to the region not being passed in in 1520 (well and also no testing for ops with regions with inference). This was being conservative due to lack of evaluation. Change to pass in regions and I think we can remove the conservative checks here. Ideally we had a test that exercised inference with regions too.

[phisiart]

Pass odsState.regions to inferReturnTypes() and update relevant test (op-result.td).

[phisiart]

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1586

Now we pass odsState.regions to inferReturnTypes().

Q: Could you expand on what "conservative checks" mean? I'm having some trouble understanding what checks to remove.

Q: op-result.td is updated to reflect the change. Would that be enough test coverage? If not, could you provide some guidance on where to add additional test cases?

Thanks!

[jpienaar]

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1586

Sure, the check is the one you removed below. I call it conservative as there isn't much technical reason behind the exclusion except I didn't have have a good end to end usage which showed if API was good.

Ideally we'd like an execution test with an op, but nothing much was missing that I can think of that a test would show beyond this. So sufficient.

[phisiart]

I don't have commit access. Could you submit this patch for me? Thanks!

mlir/test/mlir-tblgen/op-result.td

 // CHECK-LABEL: OpB definitions
 // CHECK: void OpB::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type y, ::mlir::Value x)
 // CHECK:   odsState.addTypes(y);
 // CHECK: void OpB::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Value x)
 // CHECK:   ::llvm::SmallVector<::mlir::Type, 2> inferredReturnTypes;
 // CHECK:   if (::mlir::succeeded(OpB::inferReturnTypes(odsBuilder.getContext(),
 // CHECK:             odsState.location, odsState.operands,
 // CHECK:             odsState.attributes.getDictionary(odsState.getContext()),
-// CHECK:             /*regions=*/{}, inferredReturnTypes)))
+// CHECK:             odsState.regions, inferredReturnTypes)))
 // CHECK:     odsState.addTypes(inferredReturnTypes);
 
 def OpC : NS_Op<"three_normal_result_op", []> {
   let results = (outs I32:$x, /*unnamed*/I32, I32:$z);
 }
 
 // CHECK-LABEL: OpC definitions
 // CHECK:       void OpC::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type x, ::mlir::Type resultType1, ::mlir::Type z)

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

genCodeForAddingArgAndRegionForBuilder(body, inferredAttributes,
                                                AttrParamKind::UnwrappedValue);
 
     // Push all result types to the operation state
 
     if (inferType) {
       // Generate builder that infers type too.
       // TODO: Subsume this with general checking if type can be
       // inferred automatically.
-      // TODO: Expand to handle regions.
       body << formatv(R"(
         ::llvm::SmallVector<::mlir::Type, 2> inferredReturnTypes;
         if (::mlir::succeeded({0}::inferReturnTypes(odsBuilder.getContext(),
                       {1}.location, {1}.operands,
                       {1}.attributes.getDictionary({1}.getContext()),
-                      /*regions=*/{{}, inferredReturnTypes)))
+                      {1}.regions, inferredReturnTypes)))
           {1}.addTypes(inferredReturnTypes);
         else
           ::llvm::report_fatal_error("Failed to infer result type(s).");)",
                       opClass.getClassName(), builderOpState);
       return;
     }
 
     switch (paramKind) {
auto emit = [&](AttrParamKind attrType, TypeParamKind paramKind,
     }
     llvm_unreachable("unhandled TypeParamKind");
   };
 
   // Some of the build methods generated here may be ambiguous, but TableGen's
   // ambiguous function detection will elide those ones.
   for (auto attrType : attrBuilderType) {
     emit(attrType, TypeParamKind::Separate, /*inferType=*/false);
-    if (canInferType(op) && op.getNumRegions() == 0)
+    if (canInferType(op))
       emit(attrType, TypeParamKind::None, /*inferType=*/true);
     emit(attrType, TypeParamKind::Collective, /*inferType=*/false);
   }
 }
 
 void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder() {
   int numResults = op.getNumResults();