[mlir][llvmir] Cleaned up MetadataOp.

Added NoTerminator trait, and created a single builder
that adds a block into the region at operation construction.
Added custom assembly parser that automatically adds the body
block, when the region appears to be empty to parseRegion().

Differential Revision: https://reviews.llvm.org/D141160

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index f426d69..63c4fea 100644 (file)
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -989,7 +989,7 @@ def LLVM_AddressOfOp : LLVM_Op<"mlir.addressof",
 }
 
 def LLVM_MetadataOp : LLVM_Op<"metadata", [
-   NoRegionArguments, SymbolTable, Symbol
+   NoRegionArguments, NoTerminator, SymbolTable, Symbol
 ]> {
   let arguments = (ins
     SymbolNameAttr:$sym_name
@@ -1002,11 +1002,19 @@ def LLVM_MetadataOp : LLVM_Op<"metadata", [
       llvm.metadata @metadata {
         llvm.access_group @group1
         llvm.access_group @group2
-        llvm.return
       }
   }];
   let regions = (region SizedRegion<1>:$body);
-  let assemblyFormat = "$sym_name attr-dict-with-keyword $body";
+
+  let skipDefaultBuilders = 1;
+
+  let builders = [
+    OpBuilder<(ins "StringRef":$symName,
+        CArg<"bool", "true">:$createBodyBlock,
+        CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes)>
+  ];
+
+  let hasCustomAssemblyFormat = 1;
   let hasRegionVerifier = 1;
 }
 
@@ -1060,7 +1068,6 @@ def LLVM_AliasScopeMetadataOp : LLVM_Op<"alias_scope", [
           llvm.alias_scope_domain @domain { description = "Optional domain description"}
           llvm.alias_scope @scope1 { domain = @domain }
           llvm.alias_scope @scope2 { domain = @domain, description = "Optional scope description" }
-          llvm.return
         }
       }
 
@@ -1104,7 +1111,6 @@ def LLVM_TBAARootMetadataOp : LLVM_Op<"tbaa_root", [
     Example:
       llvm.metadata @tbaa {
         llvm.tbaa_root @tbaa_root_0 {identity = "Simple C/C++ TBAA"}
-        llvm.return
       }
 
     See the following link for more details:
@@ -1157,7 +1163,6 @@ def LLVM_TBAATypeDescriptorOp : LLVM_Op<"tbaa_type_desc", [
             members = [@tbaa_type_desc_5, @tbaa_type_desc_5],
             offsets = array<i64: 0, 4>
         }
-        llvm.return
       }
 
     See the following link for more details:
@@ -1227,7 +1232,6 @@ def LLVM_TBAATagOp : LLVM_Op<"tbaa_tag", [
             base_type = @tbaa_type_desc_6,
             offset = 0 : i64
         }
-        llvm.return
       }
 
     See the following link for more details:

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index bb47aeb..adab5fe 100644 (file)
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2593,6 +2593,45 @@ OpFoldResult LLVM::GEPOp::fold(FoldAdaptor adaptor) {
 // Utilities for LLVM::MetadataOp
 //===----------------------------------------------------------------------===//
 
+void MetadataOp::build(OpBuilder &builder, OperationState &result,
+                       StringRef symName, bool createBodyBlock,
+                       ArrayRef<NamedAttribute> attributes) {
+  result.addAttribute(getSymNameAttrName(result.name),
+                      builder.getStringAttr(symName));
+  result.attributes.append(attributes.begin(), attributes.end());
+  Region *body = result.addRegion();
+  if (createBodyBlock)
+    body->emplaceBlock();
+}
+
+ParseResult MetadataOp::parse(OpAsmParser &parser, OperationState &result) {
+  StringAttr symName;
+  if (parser.parseSymbolName(symName, getSymNameAttrName(result.name),
+                             result.attributes) ||
+      parser.parseOptionalAttrDictWithKeyword(result.attributes))
+    return failure();
+
+  Region *bodyRegion = result.addRegion();
+  if (parser.parseRegion(*bodyRegion))
+    return failure();
+
+  // If the region appeared to be empty to parseRegion(),
+  // add the body block explicitly.
+  if (bodyRegion->empty())
+    bodyRegion->emplaceBlock();
+
+  return success();
+}
+
+void MetadataOp::print(OpAsmPrinter &printer) {
+  printer << ' ';
+  printer.printSymbolName(getSymName());
+  printer.printOptionalAttrDictWithKeyword((*this)->getAttrs(),
+                                           {getSymNameAttrName().getValue()});
+  printer << ' ';
+  printer.printRegion(getBody());
+}
+
 namespace {
 // A node of the TBAA graph.
 struct TBAAGraphNode {

diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index b85fae1..a11ff55 100644 (file)
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -367,8 +367,6 @@ MetadataOp ModuleImport::getTBAAMetadataOp() {
 
   builder.setInsertionPointToEnd(mlirModule.getBody());
   tbaaMetadataOp = builder.create<MetadataOp>(loc, getTBAAMetadataOpName());
-  builder.createBlock(&tbaaMetadataOp.getBody());
-  builder.create<ReturnOp>(loc, Value{});
 
   return tbaaMetadataOp;
 }
@@ -535,9 +533,9 @@ LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
     return true;
   };
 
-  // Insert new operations before the terminator.
+  // Insert new operations at the end of the MetadataOp.
   OpBuilder::InsertionGuard guard(builder);
-  builder.setInsertionPoint(&getTBAAMetadataOp().getBody().back().back());
+  builder.setInsertionPointToEnd(&getTBAAMetadataOp().getBody().back());
   StringAttr metadataOpName = SymbolTable::getSymbolName(getTBAAMetadataOp());
 
   // On the first walk, create SymbolRefAttr's and map them
@@ -614,7 +612,7 @@ LogicalResult ModuleImport::processTBAAMetadata(const llvm::MDNode *node) {
 
 LogicalResult ModuleImport::convertMetadata() {
   OpBuilder::InsertionGuard guard(builder);
-  builder.setInsertionPoint(mlirModule.getBody(), mlirModule.getBody()->end());
+  builder.setInsertionPointToEnd(mlirModule.getBody());
   for (const llvm::Function &func : llvmModule->functions())
     for (const llvm::Instruction &inst : llvm::instructions(func)) {
       llvm::AAMDNodes nodes = inst.getAAMetadata();

diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index 1a50afa..1593906 100644 (file)
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -867,7 +867,6 @@ module {
       llvm.return
   }
   llvm.metadata @metadata {
-    llvm.return
   }
 }
 
@@ -936,7 +935,6 @@ module {
       llvm.return
   }
   llvm.metadata @metadata {
-    llvm.return
   }
 }
 
@@ -949,7 +947,6 @@ module {
       llvm.return
   }
   llvm.metadata @metadata {
-    llvm.return
   }
 }
 
@@ -964,7 +961,6 @@ module {
   llvm.metadata @metadata {
     llvm.alias_scope_domain @domain
     llvm.alias_scope @scope { domain = @domain }
-    llvm.return
   }
 }
 
@@ -998,7 +994,6 @@ module {
   }
   llvm.metadata @metadata {
     llvm.access_group @group
-    llvm.return
   }
 }
 
@@ -1012,7 +1007,6 @@ module {
   }
   llvm.metadata @metadata {
     llvm.access_group @group
-    llvm.return
   }
 }
 

diff --git a/mlir/test/Dialect/LLVMIR/roundtrip.mlir b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
index 515215b..a37b154 100644 (file)
--- a/mlir/test/Dialect/LLVMIR/roundtrip.mlir
+++ b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
@@ -504,7 +504,6 @@ module {
   llvm.metadata @metadata attributes {test_attribute} {
     // CHECK: llvm.access_group @group1
     llvm.access_group @group1
-    llvm.return
   }
 }
 

diff --git a/mlir/test/Dialect/LLVMIR/tbaa-invalid.mlir b/mlir/test/Dialect/LLVMIR/tbaa-invalid.mlir
index 6e7f668..0513dc7 100644 (file)
--- a/mlir/test/Dialect/LLVMIR/tbaa-invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/tbaa-invalid.mlir
@@ -4,7 +4,6 @@ module {
   llvm.metadata @__tbaa {
     llvm.tbaa_root @tbaa_root_0 {id = "Simple C/C++ TBAA"}
     llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_0, base_type = @tbaa_root_0, offset = 0 : i64}
-    llvm.return
   }
   llvm.func @tbaa(%arg0: !llvm.ptr) {
     %0 = llvm.mlir.constant(1 : i8) : i8
@@ -34,7 +33,6 @@ module {
   }
   llvm.metadata @metadata {
     llvm.access_group @group1
-    llvm.return
   }
 }
 
@@ -48,7 +46,6 @@ module {
     llvm.return
   }
   llvm.metadata @metadata {
-    llvm.return
   }
 }
 
@@ -74,7 +71,6 @@ llvm.func @tbaa() {
 module {
   llvm.metadata @__tbaa {
     llvm.tbaa_root @tbaa_root_0 {id = "Simple C/C++ TBAA"}
-    llvm.return
   }
 
   llvm.func @tbaa() {
@@ -89,7 +85,6 @@ module {
 module {
   llvm.metadata @__tbaa {
     llvm.tbaa_root @tbaa_root_0 {id = "Simple C/C++ TBAA"}
-    llvm.return
   }
 
   llvm.func @tbaa() {
@@ -105,7 +100,6 @@ module {
   llvm.metadata @__tbaa {
     // expected-error@below {{expected non-empty "identity"}}
     llvm.tbaa_root @tbaa_root_0 {id = ""}
-    llvm.return
   }
 }
 
@@ -117,7 +111,6 @@ module {
       "llvm.tbaa_type_desc"() {identity = "omnipotent char", members = [@tbaa_root_0], offsets = array<i64: 0>, sym_name = "tbaa_type_desc_1"} : () -> ()
     // expected-error@below {{expected the same number of elements in "members" and "offsets": 2 != 1}}
       "llvm.tbaa_type_desc"() {identity = "agg_t", members = [@tbaa_type_desc_1, @tbaa_type_desc_1], offsets = array<i64: 0>, sym_name = "tbaa_type_desc_2"} : () -> ()
-      "llvm.return"() : () -> ()
     }) {sym_name = "__tbaa"} : () -> ()
   }) : () -> ()
 
@@ -128,7 +121,6 @@ module {
     llvm.tbaa_root @tbaa_root_0 {id = "Simple C/C++ TBAA"}
     // expected-error@below {{expected "base_type" to reference a symbol from 'llvm.metadata @__tbaa' defined by either 'llvm.tbaa_root' or 'llvm.tbaa_type_desc' while it references '@tbaa_root_2'}}
     llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_0, base_type = @tbaa_root_2, offset = 0 : i64}
-    llvm.return
   }
 }
 
@@ -139,7 +131,6 @@ module {
     llvm.tbaa_root @tbaa_root_0 {id = "Simple C/C++ TBAA"}
     // expected-error@below {{expected "access_type" to reference a symbol from 'llvm.metadata @__tbaa' defined by either 'llvm.tbaa_root' or 'llvm.tbaa_type_desc' while it references '@tbaa_root_2'}}
     llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_2, base_type = @tbaa_root_0, offset = 0 : i64}
-    llvm.return
   }
 }
 
@@ -152,7 +143,6 @@ module {
     llvm.tbaa_type_desc @tbaa_type_desc_2 {id = "long long", members = {<@tbaa_type_desc_1, 0>}}
     // expected-error@below {{expected "members" to reference a symbol from 'llvm.metadata @__tbaa' defined by either 'llvm.tbaa_root' or 'llvm.tbaa_type_desc' while it references '@tbaa_type_desc_4'}}
     llvm.tbaa_type_desc @tbaa_type_desc_3 {id = "agg2_t", members = {<@tbaa_type_desc_2, 0>, <@tbaa_type_desc_4, 8>}}
-    llvm.return
   }
 }
 
@@ -164,7 +154,6 @@ module {
     llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_0, base_type = @tbaa_root_0, offset = 0 : i64}
     // expected-error@below {{expected "access_type" to reference a symbol from 'llvm.metadata @__tbaa' defined by either 'llvm.tbaa_root' or 'llvm.tbaa_type_desc' while it references '@tbaa_tag_1'}}
     llvm.tbaa_tag @tbaa_tag_2 {access_type = @tbaa_tag_1, base_type = @tbaa_root_0, offset = 0 : i64}
-    llvm.return
   }
 }
 
@@ -176,6 +165,5 @@ module {
     llvm.tbaa_root @tbaa_root_0 {id = "Simple C/C++ TBAA"}
     llvm.tbaa_type_desc @tbaa_type_desc_1 {id = "omnipotent char", members = {<@tbaa_type_desc_2, 0>}}
     llvm.tbaa_type_desc @tbaa_type_desc_2 {id = "long long", members = {<@tbaa_type_desc_1, 0>}}
-    llvm.return
   }
 }

diff --git a/mlir/test/Dialect/LLVMIR/tbaa-roundtrip.mlir b/mlir/test/Dialect/LLVMIR/tbaa-roundtrip.mlir
index 748cbbe..5bf38e3 100644 (file)
--- a/mlir/test/Dialect/LLVMIR/tbaa-roundtrip.mlir
+++ b/mlir/test/Dialect/LLVMIR/tbaa-roundtrip.mlir
@@ -6,7 +6,6 @@ module {
     llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_0, base_type = @tbaa_root_0, offset = 0 : i64}
     llvm.tbaa_root @tbaa_root_2 {id = "Other language TBAA"}
     llvm.tbaa_tag @tbaa_tag_3 {access_type = @tbaa_root_2, base_type = @tbaa_root_2, offset = 0 : i64}
-    llvm.return
   }
   llvm.func @tbaa1(%arg0: !llvm.ptr, %arg1: !llvm.ptr) {
     %0 = llvm.mlir.constant(1 : i8) : i8
@@ -21,7 +20,6 @@ module {
 // CHECK:             llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_0, base_type = @tbaa_root_0, offset = 0 : i64}
 // CHECK:             llvm.tbaa_root @tbaa_root_2 {id = "Other language TBAA"}
 // CHECK:             llvm.tbaa_tag @tbaa_tag_3 {access_type = @tbaa_root_2, base_type = @tbaa_root_2, offset = 0 : i64}
-// CHECK:             llvm.return
 // CHECK:           }
 // CHECK:           llvm.func @tbaa1(%[[VAL_0:.*]]: !llvm.ptr, %[[VAL_1:.*]]: !llvm.ptr) {
 // CHECK:             %[[VAL_2:.*]] = llvm.mlir.constant(1 : i8) : i8
@@ -40,7 +38,6 @@ module {
     llvm.tbaa_type_desc @tbaa_type_desc_5 {id = "int", members = {<@tbaa_type_desc_1, 0>}}
     llvm.tbaa_type_desc @tbaa_type_desc_6 {id = "agg1_t", members = {<@tbaa_type_desc_5, 0>, <@tbaa_type_desc_5, 4>}}
     llvm.tbaa_tag @tbaa_tag_7 {access_type = @tbaa_type_desc_5, base_type = @tbaa_type_desc_6, offset = 0 : i64}
-    llvm.return
   }
   llvm.func @tbaa2(%arg0: !llvm.ptr, %arg1: !llvm.ptr) {
     %0 = llvm.mlir.constant(0 : i32) : i32
@@ -63,7 +60,6 @@ module {
 // CHECK:             llvm.tbaa_type_desc @tbaa_type_desc_5 {id = "int", members = {<@tbaa_type_desc_1, 0>}}
 // CHECK:             llvm.tbaa_type_desc @tbaa_type_desc_6 {id = "agg1_t", members = {<@tbaa_type_desc_5, 0>, <@tbaa_type_desc_5, 4>}}
 // CHECK:             llvm.tbaa_tag @tbaa_tag_7 {access_type = @tbaa_type_desc_5, base_type = @tbaa_type_desc_6, offset = 0 : i64}
-// CHECK:             llvm.return
 // CHECK:           }
 // CHECK:           llvm.func @tbaa2(%[[VAL_0:.*]]: !llvm.ptr, %[[VAL_1:.*]]: !llvm.ptr) {
 // CHECK:             %[[VAL_2:.*]] = llvm.mlir.constant(0 : i32) : i32
@@ -82,7 +78,6 @@ module {
     llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_0, base_type = @tbaa_root_0, offset = 0 : i64}
     llvm.tbaa_root @tbaa_root_2 {id = "Other language TBAA"}
     llvm.tbaa_tag @tbaa_tag_3 {access_type = @tbaa_root_2, base_type = @tbaa_root_2, offset = 0 : i64}
-    llvm.return
   }
   llvm.func @tbaa1(%arg0: !llvm.ptr) {
     %0 = llvm.mlir.constant(1 : i8) : i8
@@ -96,7 +91,6 @@ module {
 // CHECK:             llvm.tbaa_tag @tbaa_tag_1 {access_type = @tbaa_root_0, base_type = @tbaa_root_0, offset = 0 : i64}
 // CHECK:             llvm.tbaa_root @tbaa_root_2 {id = "Other language TBAA"}
 // CHECK:             llvm.tbaa_tag @tbaa_tag_3 {access_type = @tbaa_root_2, base_type = @tbaa_root_2, offset = 0 : i64}
-// CHECK:             llvm.return
 // CHECK:           }
 // CHECK:           llvm.func @tbaa1(%[[VAL_0:.*]]: !llvm.ptr) {
 // CHECK:             %[[VAL_1:.*]] = llvm.mlir.constant(1 : i8) : i8

diff --git a/mlir/test/Target/LLVMIR/Import/tbaa.ll b/mlir/test/Target/LLVMIR/Import/tbaa.ll
index 8d23b56..7daf7d2 100644 (file)
--- a/mlir/test/Target/LLVMIR/Import/tbaa.ll
+++ b/mlir/test/Target/LLVMIR/Import/tbaa.ll
@@ -7,7 +7,6 @@
 ; CHECK-NEXT:    llvm.tbaa_tag @[[T0:tbaa_tag_[0-9]+]] {access_type = @[[R0]], base_type = @[[R0]], offset = 0 : i64}
 ; CHECK-NEXT:    llvm.tbaa_root @[[R1:tbaa_root_[0-9]+]] {id = "Other language TBAA"}
 ; CHECK-NEXT:    llvm.tbaa_tag @[[T1:tbaa_tag_[0-9]+]] {access_type = @[[R1]], base_type = @[[R1]], offset = 0 : i64}
-; CHECK-NEXT:    llvm.return
 ; CHECK-NEXT:  }
 ; CHECK:       llvm.func @tbaa1
 ; CHECK:         llvm.store %{{.*}}, %{{.*}} {
@@ -38,7 +37,6 @@ define dso_local void @tbaa1(ptr %0, ptr %1) {
 ; CHECK-NEXT:    llvm.tbaa_tag @[[T1:tbaa_tag_[0-9]+]] {access_type = @[[D3:tbaa_type_desc_[0-9]+]], base_type = @[[D4:tbaa_type_desc_[0-9]+]], offset = 0 : i64}
 ; CHECK-NEXT:    llvm.tbaa_type_desc @[[D3]] {id = "int", members = {<@[[D0]], 0>}}
 ; CHECK-NEXT:    llvm.tbaa_type_desc @[[D4]] {id = "agg1_t", members = {<@[[D3]], 0>, <@[[D3]], 4>}}
-; CHECK-NEXT:    llvm.return
 ; CHECK-NEXT:  }
 ; CHECK:       llvm.func @tbaa2
 ; CHECK:         llvm.load %{{.*}} {

diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 17f649b..18331a2 100644 (file)
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -1889,7 +1889,6 @@ module {
   llvm.metadata @metadata {
     llvm.access_group @group1
     llvm.access_group @group2
-    llvm.return
   }
 }
 
@@ -1920,7 +1919,6 @@ module {
     llvm.alias_scope @scope1 { domain = @domain, description = "The first scope" }
     llvm.alias_scope @scope2 { domain = @domain }
     llvm.alias_scope @scope3 { domain = @domain }
-    llvm.return
   }
 }
 

diff --git a/mlir/test/Target/LLVMIR/tbaa.mlir b/mlir/test/Target/LLVMIR/tbaa.mlir
index 83474c1..0ed23ed 100644 (file)
--- a/mlir/test/Target/LLVMIR/tbaa.mlir
+++ b/mlir/test/Target/LLVMIR/tbaa.mlir
@@ -10,7 +10,6 @@ module {
     llvm.tbaa_type_desc @tbaa_type_desc_5 {id = "int", members = {<@tbaa_type_desc_1, 0>}}
     llvm.tbaa_type_desc @tbaa_type_desc_6 {id = "agg1_t", members = {<@tbaa_type_desc_5, 0>, <@tbaa_type_desc_5, 4>}}
     llvm.tbaa_tag @tbaa_tag_7 {access_type = @tbaa_type_desc_5, base_type = @tbaa_type_desc_6, offset = 0 : i64, constant}
-    llvm.return
   }
   llvm.func @tbaa2(%arg0: !llvm.ptr, %arg1: !llvm.ptr) {
     %0 = llvm.mlir.constant(0 : i32) : i32