[mlir][ods] Allow type attribute/operand for 0 result ops prefixed

[jpienaar]

Summary

Without results, there is no getType injected and so generating one in prefixed form doesn't result in any failures during C++ compilation.

[rriddle]

LG

flang/include/flang/Optimizer/Dialect/FIROps.td

def fir_GlobalOp : fir_Op<"global", [IsolatedFromAbove, Symbol]> {
 
   let extraClassDeclaration = [{
     static constexpr llvm::StringRef symbolAttrNameStr() { return "symref"; }
     static constexpr llvm::StringRef getConstantAttrNameStr() {
       return "constant";
     }
     static constexpr llvm::StringRef linkageAttrName() { return "linkName"; }
 
-    /// The printable type of the global
-    mlir::Type getType() {
-      return typeAttr().getValue();
-    }
-
     /// The semantic type of the global
     mlir::Type resultType();
 
     /// Return the initializer attribute if it exists, or a null attribute.
     Attribute getValueOrNull() { return getInitVal().getValueOr(Attribute()); }
 
     /// Append the next initializer value to the `GlobalOp` to construct
     /// the variable's initial value.

flang/lib/Optimizer/Dialect/FIROps.cpp

if (succeeded(parser.parseOptionalKeyword("constant"))) {
     // if "constant" keyword then mark this as a constant, not a variable
     result.addAttribute("constant", builder.getUnitAttr());
   }
 
   mlir::Type globalType;
   if (parser.parseColonType(globalType))
     return mlir::failure();
 
-  result.addAttribute(fir::GlobalOp::typeAttrName(result.name),
+  result.addAttribute(fir::GlobalOp::getTypeAttrName(result.name),
                       mlir::TypeAttr::get(globalType));
 
   if (simpleInitializer) {
     result.addRegion();
   } else {
     // Parse the optional initializer body.
     auto parseResult = parser.parseOptionalRegion(
         *result.addRegion(), /*arguments=*/llvm::None, /*argTypes=*/llvm::None);
void fir::GlobalOp::appendInitialValue(mlir::Operation *op) {
   getBlock().getOperations().push_back(op);
 }
 
 void fir::GlobalOp::build(mlir::OpBuilder &builder, OperationState &result,
                           StringRef name, bool isConstant, Type type,
                           Attribute initialVal, StringAttr linkage,
                           ArrayRef<NamedAttribute> attrs) {
   result.addRegion();
-  result.addAttribute(typeAttrName(result.name), mlir::TypeAttr::get(type));
+  result.addAttribute(getTypeAttrName(result.name), mlir::TypeAttr::get(type));
   result.addAttribute(mlir::SymbolTable::getSymbolAttrName(),
                       builder.getStringAttr(name));
   result.addAttribute(symbolAttrNameStr(),
                       SymbolRefAttr::get(builder.getContext(), name));
   if (isConstant)
     result.addAttribute(getConstantAttrName(result.name),
                         builder.getUnitAttr());
   if (initialVal)

flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp

TEST_F(FIRBuilderTest, createGlobal1) {
   auto builder = getBuilder();
   auto loc = builder.getUnknownLoc();
   auto i64Type = IntegerType::get(builder.getContext(), 64);
   auto global = builder.createGlobal(
       loc, i64Type, "global1", builder.createInternalLinkage(), {}, true);
   EXPECT_TRUE(mlir::isa<fir::GlobalOp>(global));
   EXPECT_EQ("global1", global.getSymName());
   EXPECT_TRUE(global.getConstant().hasValue());
-  EXPECT_EQ(i64Type, global.type());
+  EXPECT_EQ(i64Type, global.getType());
   EXPECT_TRUE(global.getLinkName().hasValue());
   EXPECT_EQ(builder.createInternalLinkage().getValue(),
       global.getLinkName().getValue());
   EXPECT_FALSE(global.getInitVal().hasValue());
 
   auto g1 = builder.getNamedGlobal("global1");
   EXPECT_EQ(global, g1);
   auto g2 = builder.getNamedGlobal("global7");
   EXPECT_EQ(nullptr, g2);
   auto g3 = builder.getNamedGlobal("");
   EXPECT_EQ(nullptr, g3);
 }
 
 TEST_F(FIRBuilderTest, createGlobal2) {
   auto builder = getBuilder();
   auto loc = builder.getUnknownLoc();
   auto i32Type = IntegerType::get(builder.getContext(), 32);
   auto attr = builder.getIntegerAttr(i32Type, 16);
   auto global = builder.createGlobal(
       loc, i32Type, "global2", builder.createLinkOnceLinkage(), attr, false);
   EXPECT_TRUE(mlir::isa<fir::GlobalOp>(global));
   EXPECT_EQ("global2", global.getSymName());
   EXPECT_FALSE(global.getConstant().hasValue());
-  EXPECT_EQ(i32Type, global.type());
+  EXPECT_EQ(i32Type, global.getType());
   EXPECT_TRUE(global.getInitVal().hasValue());
   EXPECT_TRUE(global.getInitVal().getValue().isa<mlir::IntegerAttr>());
   EXPECT_EQ(
       16, global.getInitVal().getValue().cast<mlir::IntegerAttr>().getValue());
   EXPECT_TRUE(global.getLinkName().hasValue());
   EXPECT_EQ(builder.createLinkOnceLinkage().getValue(),
       global.getLinkName().getValue());
 }
TEST_F(FIRBuilderTest, createStringLiteral) {
   auto addr = charBox->getBuffer();
   EXPECT_TRUE(mlir::isa<fir::AddrOfOp>(addr.getDefiningOp()));
   auto addrOp = dyn_cast<fir::AddrOfOp>(addr.getDefiningOp());
   auto symbol = addrOp.getSymbol().getRootReference().getValue();
   auto global = builder.getNamedGlobal(symbol);
   EXPECT_EQ(builder.createLinkOnceLinkage().getValue(),
       global.getLinkName().getValue());
   EXPECT_EQ(fir::CharacterType::get(builder.getContext(), 1, strValue.size()),
-      global.type());
+      global.getType());
 
   auto stringLitOps = global.getRegion().front().getOps<fir::StringLitOp>();
   EXPECT_TRUE(llvm::hasSingleElement(stringLitOps));
   for (auto stringLit : stringLitOps) {
     EXPECT_EQ(16, stringLit.getSize().cast<mlir::IntegerAttr>().getValue());
     EXPECT_TRUE(stringLit.getValue().isa<StringAttr>());
     EXPECT_EQ(strValue, stringLit.getValue().dyn_cast<StringAttr>().getValue());
   }

mlir/lib/TableGen/Operator.cpp

getGetterOrSetterNames(bool isGetter, const Operator &op, StringRef name) {
   //
   // There are a little bit more invasive checks possible for cases where not
   // all ops have the trait that would cause overlap. For many cases here,
   // renaming would be better (e.g., we can only guard in limited manner against
   // methods from traits and interfaces here, so avoiding these in op definition
   // is safer).
   auto skip = [&](StringRef newName) {
     bool shouldSkip = newName == "getAttributeNames" ||
-                      newName == "getAttributes" || newName == "getOperation" ||
-                      newName == "getType";
+                      newName == "getAttributes" || newName == "getOperation";
     if (newName == "getOperands") {
       // To reduce noise, skip generating the prefixed form and the warning if
       // $operands correspond to single variadic argument.
       if (op.getNumOperands() == 1 && op.getNumVariableLengthOperands() == 1)
         return true;
       shouldSkip = true;
     }
     if (newName == "getRegions") {
       if (op.getNumRegions() == 1 && op.getNumVariadicRegions() == 1)
         return true;
       shouldSkip = true;
     }
+    if (newName == "getType") {
+      if (op.getNumResults() == 0)
+        return false;
+      shouldSkip = true;
+    }
     if (!shouldSkip)
       return false;
 
     // This note could be avoided where the final function generated would
     // have been identical. But preferably in the op definition avoiding using
     // the generic name and then getting a more specialize type is better.
     PrintNote(op.getLoc(),
               "Skipping generation of prefixed accessor `" + newName +