[MLIR] Add mlir::TypedValue

[Tyker]

Summary

mlir::TypedValue is a wrapper class for mlir::Values with a known type
getType will return the known type and all assignments will be asserted
Also the tablegen Operation generator was adapted to use mlir::TypedValue
when appropriate
If an op can only take one type for a specific fields, getting this field
will return a TypedValue of the correct type. so there is no need to cast the type.
I also think TypedValue can be a useful to express directly what an API is expecting or providing
and remove the need for to cast the return of .getType() its Values when using that API.

for example:

/// expressed directly in the API that that the parameter should be a Value of IntegerType.
/// And the return will still be of IntegerType.
TypedValue<IntegerType> maybeCast(TypedValue<IntegerType> )

Value v;
auto v2 = maybeCast(v) /// asserts that the Type is IntergerType
assert(v2.getType().getWidth() == 8); /// no need to do .cast<IntegerType>()

/// Assuming MyOp is defined to operate on TensorType exclusively
MyOp.getLHS().getType() // is a TensorType

/// Not just for APIs
Value getValue()

/// express directly that the expected type of v is IntegerType
TypedValue<IntegerType> temporary = getValue()

/// express that this should only store Values of type IntegerType
DenseMap<mlir::Operations*, TypedValue<IntegerType>>

[mehdi_amini]

Can you elaborate more on the motivation in the description?
Right now you're touching on it with:

I also think TypedValue can be a useful as to express directly what is expected and remove the need for to cast its Values.

But I'm not sure what you mean, if you could also add examples of what kind of API would take advantage of this?

[mehdi_amini]

mlir/include/mlir/IR/Value.h

423

Nit: "checked" -> "asserted", there won't be any check in non-assert build.

[Tyker]

In D132010#3728346, @mehdi_amini wrote:

Can you elaborate more on the motivation in the description?
Right now you're touching on it with:

I also think TypedValue can be a useful as to express directly what is expected and remove the need for to cast its Values.

But I'm not sure what you mean, if you could also add examples of what kind of API would take advantage of this?

I added a few examples.

[rriddle]

How many places in-tree would benefit from this? It's hard to gauge what the value of this is from the patch as-is.

mlir/include/mlir/IR/Value.h

425

The ValueClass is never anything other than Value, why have this as a template arg?

456–467

Why not just use std::conditional_t?

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1158–1162

This should be the expected state of ODS (having qualified types), we shouldn't guard against this ever.

[mehdi_amini]

assert(v2.getType().getWidth() == 8); /// no need to do .cast<IntegerType>()

/// Assuming MyOp is defined to operate on TensorType exclusively
MyOp.getLHS().getType() // is a TensorType

Ah I like this! The API is indeed quite nice when getType does not need to be casted... So I'm supportive here if River is as well :)

(You have a typo in multiple place you wrote IntergerType)

[Tyker]

In D132010#3729321, @rriddle wrote:

How many places in-tree would benefit from this? It's hard to gauge what the value of this is from the patch as-is.

When ever an operand of an op can operate on only one type. this helps by removing the need for casting the result of getType
a build of all the in-tree dialects with the patch emits 6413 TypedValue with 3657 being TypedValue<Type>, so 42% of operand have a known type

mlir/include/mlir/IR/Value.h

425

This was originally to support other Kinds of Value, like TypedValue<IntergerType, BlockArgument>. but it is rare (at least for me) to cast between Values so I removed it.

456–467

yes it is simpler

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1158–1162

I agree that it shouldn't happen. but during testing I found it happens. so this protection is needed. at least until the places where types are not fully qualified become fully qualified.

tablegen should check that the provided type string are fully qualified types (starting with "::").
after looking into it pdl::OperationType seems to be the only type that is causing issues. but I didn't find a way to fix it.

example:

/home/tyker/opensource/llvm-project/build-release/tools/mlir/test/lib/Dialect/Transform/TestTransformDialectExtension.h.inc:945:22: error: unknown type name 'OperationType'; did you mean 'pdl::OperationType'?
  ::mlir::TypedValue<OperationType> getOperand();
                     ^~~~~~~~~~~~~
                     pdl::OperationType

[Tyker]

In D132010#3730193, @mehdi_amini wrote:

assert(v2.getType().getWidth() == 8); /// no need to do .cast<IntegerType>()

/// Assuming MyOp is defined to operate on TensorType exclusively
MyOp.getLHS().getType() // is a TensorType

Ah I like this! The API is indeed quite nice when getType does not need to be casted... So I'm supportive here if River is as well :)

:)

(You have a typo in multiple place you wrote IntergerType)

The description was updated sorry for the typos

[rriddle]

mlir/include/mlir/IR/Value.h

422–424

Can you reflow these comments?

430

This assert doesn't look useful, this should be statically guaranteed by the fact that the argument of the function is already Ty.

437–440

Is this used anywhere? Why not force the user to cast the Value before assignment?

483–487

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1158–1162

Can you file an issue for this and add a link here? We ideally shouldn't be baking in checks for things we consider bugs.

1387–1388

[Tyker]

mlir/include/mlir/IR/Value.h

430

Yeah, setType should be taking an mlir::Type and checking it.

437–440

This is here if someone wants to store a TypedValue somewhere. which can happens by default when auto.

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1158–1162

here is the issue: https://github.com/llvm/llvm-project/issues/57279.

[rriddle]

LGTM in general, but I'd like comments addressed first.

mlir/include/mlir/IR/Value.h

437–440

This breaks away from the precedent set everywhere else, how annoying is it to remove this?

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

1158–1162

Can you link to it in a comment?

[Tyker]

This comment has been deleted.

mlir/include/mlir/IR/Value.h

437–440

It is cheap to remove this since there is only one failure in the upstream MLIR.
what precedent does it break away from ? not overloading operator= ?

I am not sure if it should be removed anyway. because removing this would limit TypedValue variables or containers which I think can be useful. now if you think TypedValue variables should be discouraged. I can remove support for it.

[Tyker]

pushed as 688c51a5acc53b456014e53663051476d825e896

[mscuttari]

mlir/include/mlir/IR/Value.h

434

This is making the lookup method of BlockAndValueMapping to crash when the input is a TypedValue with a type different from the mapped value. The Ty template becomes TypedValue and the wrapper is instantied also for the result, which however may have a different type.
See also line 66-71 in mlir/IR/BLockAndValueMapping.h

[Tyker]

mlir/include/mlir/IR/Value.h

434

Can Values be mapped to values of different type with BlockAndValueMapping ?
Do you have an example of a crash with mlir because of this ?

[mscuttari]

mlir/include/mlir/IR/Value.h

434

I don't see why not, the BlockAndValueMapping class does not impose any constraint about that. In my projects, for example, it happens that some operations have to unwrapped during the cloning process, thus resulting in a different type.
Here is a minimum working example reproducing the problem, including also the workaround I am currently using.

[Tyker]

mlir/include/mlir/IR/Value.h

434

here is a patch to solve the issue. https://reviews.llvm.org/D133384

mlir/include/mlir/Dialect/Bufferization/TransformOps/BufferizationTransformOps.h

 //===- BufferizationTransformOps.h - Buff. transf. ops ----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_DIALECT_BUFFERIZATION_TRANSFORMOPS_BUFFERIZATIONTRANSFORMOPS_H
 #define MLIR_DIALECT_BUFFERIZATION_TRANSFORMOPS_BUFFERIZATIONTRANSFORMOPS_H
 
+#include "mlir/Dialect/PDL/IR/PDLTypes.h"
 #include "mlir/Dialect/Transform/IR/TransformInterfaces.h"
 #include "mlir/IR/OpImplementation.h"
 
 //===----------------------------------------------------------------------===//
 // Bufferization Transform Operations
 //===----------------------------------------------------------------------===//
 
 #define GET_OP_CLASSES

mlir/include/mlir/IR/Value.h

 
 /// Return the result number of this op result.
 inline unsigned OpResultImpl::getResultNumber() const {
   if (const auto *outOfLineResult = dyn_cast<OutOfLineOpResult>(this))
     return outOfLineResult->getResultNumber();
   return cast<InlineOpResult>(this)->getResultNumber();
 }
 
+/// TypedValue is a Value with a statically know type.
+/// TypedValue can be null/empty
+template <typename Ty>
+struct TypedValue : Value {
+  /// Return the known Type
+  Ty getType() { return Value::getType().template cast<Ty>(); }
+  void setType(mlir::Type ty) {
+    assert(ty.template isa<Ty>());
+    Value::setType(ty);
+  }
+
+  TypedValue(Value val) : Value(val) {
+    assert(!val || val.getType().template isa<Ty>());
+  }
+  TypedValue &operator=(const Value &other) {
+    assert(!other || other.getType().template isa<Ty>());
+    Value::operator=(other);
+    return *this;
+  }
+};
+
 } // namespace detail
 
 /// This is a value defined by a result of an operation.
 class OpResult : public Value {
 public:
   using Value::Value;
 
   static bool classof(Value value) {
     return llvm::isa<detail::OpResultImpl>(value.getImpl());
   }
 
   /// Returns the operation that owns this result.
   Operation *getOwner() const { return getImpl()->getOwner(); }
 
   /// Returns the number of this result.
   unsigned getResultNumber() const { return getImpl()->getResultNumber(); }
 
 private:
   /// Get a raw pointer to the internal implementation.
   detail::OpResultImpl *getImpl() const {
     return reinterpret_cast<detail::OpResultImpl *>(impl);
   }
 
   /// Given a number of operation results, returns the number that need to be
   /// stored inline.
   static unsigned getNumInline(unsigned numResults);
 
   /// Given a number of operation results, returns the number that need to be
   /// stored as trailing.
   static unsigned getNumTrailing(unsigned numResults);
 
   /// Allow access to constructor.
   friend Operation;
 };
 
 /// Make Value hashable.
 inline ::llvm::hash_code hash_value(Value arg) {
   return ::llvm::hash_value(arg.getImpl());
 }
 
+template <typename Ty, typename Value = mlir::Value>
+/// If Ty is mlir::Type this will select `Value` instead of having a wrapper
+/// around it. This helps resolve ambiguous conversion issues.
+using TypedValue = std::conditional_t<std::is_same_v<Ty, mlir::Type>,
+                                      mlir::Value, detail::TypedValue<Ty>>;
+
 } // namespace mlir
 
 namespace llvm {
 
 template <>
 struct DenseMapInfo<mlir::Value> {
   static mlir::Value getEmptyKey() {
     void *pointer = llvm::DenseMapInfo<void *>::getEmptyKey();

mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp

LogicalResult ConvertLaunchFuncOpToGpuRuntimeCallPattern::matchAndRewrite(
   auto module = moduleLoadCallBuilder.create(loc, rewriter, data);
   // Get the function from the module. The name corresponds to the name of
   // the kernel function.
   auto kernelName = generateKernelNameConstant(
       launchOp.getKernelModuleName().getValue(),
       launchOp.getKernelName().getValue(), loc, rewriter);
   auto function = moduleGetFunctionCallBuilder.create(
       loc, rewriter, {module.getResult(), kernelName});
-  auto zero = rewriter.create<LLVM::ConstantOp>(loc, llvmInt32Type, 0);
+  Value zero = rewriter.create<LLVM::ConstantOp>(loc, llvmInt32Type, 0);
   Value stream =
       adaptor.asyncDependencies().empty()
           ? streamCreateCallBuilder.create(loc, rewriter, {}).getResult()
           : adaptor.asyncDependencies().front();
   // Create array of pointers to kernel arguments.
   auto kernelParams = generateParamsArray(launchOp, adaptor, rewriter);
   auto nullpointer = rewriter.create<LLVM::NullOp>(loc, llvmPointerPointerType);
   Value dynamicSharedMemorySize = launchOp.dynamicSharedMemorySize()

mlir/lib/Dialect/SCF/Utils/Utils.cpp

LogicalResult mlir::loopUnrollByFactor(
   // Return if the loop body is empty.
   if (llvm::hasSingleElement(forOp.getBody()->getOperations()))
     return success();
 
   // Compute tripCount = ceilDiv((upperBound - lowerBound), step) and populate
   // 'upperBoundUnrolled' and 'stepUnrolled' for static and dynamic cases.
   OpBuilder boundsBuilder(forOp);
   auto loc = forOp.getLoc();
-  auto step = forOp.getStep();
+  Value step = forOp.getStep();
   Value upperBoundUnrolled;
   Value stepUnrolled;
   bool generateEpilogueLoop = true;
 
   auto lbCstOp = forOp.getLowerBound().getDefiningOp<arith::ConstantIndexOp>();
   auto ubCstOp = forOp.getUpperBound().getDefiningOp<arith::ConstantIndexOp>();
   auto stepCstOp = forOp.getStep().getDefiningOp<arith::ConstantIndexOp>();
   if (lbCstOp && ubCstOp && stepCstOp) {

mlir/lib/Dialect/Shape/IR/Shape.cpp

 struct CanonicalizeCastExtentTensorOperandsPattern
     : public OpRewritePattern<OpTy> {
   using OpRewritePattern<OpTy>::OpRewritePattern;
 
   LogicalResult matchAndRewrite(OpTy op,
                                 PatternRewriter &rewriter) const override {
     // Canonicalize operands.
     bool anyChange = false;
-    auto canonicalizeOperand = [&](Value operand) {
+    auto canonicalizeOperand = [&](Value operand) -> Value {
       if (auto castOp = operand.getDefiningOp<tensor::CastOp>()) {
         // Only eliminate the cast if it holds no shape information.
         bool isInformationLoosingCast =
             castOp.getType().cast<RankedTensorType>().isDynamicDim(0);
         if (isInformationLoosingCast) {
           anyChange = true;
           return castOp.getSource();
         }

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

 //===- TestDialect.h - MLIR Dialect for testing -----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines a fake 'test' dialect that can be used for testing things
 // that do not have a respective counterpart in the main source directories.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_TESTDIALECT_H
 #define MLIR_TESTDIALECT_H
 
+#include "TestTypes.h"
 #include "TestAttributes.h"
 #include "TestInterfaces.h"
 #include "mlir/Dialect/DLTI/DLTI.h"
 #include "mlir/Dialect/DLTI/Traits.h"
 #include "mlir/Dialect/Func/IR/FuncOps.h"
 #include "mlir/Dialect/Linalg/IR/Linalg.h"
 #include "mlir/Dialect/Traits.h"
 #include "mlir/IR/AsmState.h"

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

 }
 
 // We should not have a default attribute in this case.
 
 // DECL-LABEL: OpWithDefaultAndSuccessor declarations
 // DECL: static void build({{.*}}, bool dv_bool_attr, ::mlir::BlockRange succ)
 
 // DEF-LABEL: MixOperandsAndAttrs definitions
-// DEF-DAG: ::mlir::Value MixOperandsAndAttrs::operand()
-// DEF-DAG: ::mlir::Value MixOperandsAndAttrs::otherArg()
+// DEF-DAG: ::mlir::TypedValue<::mlir::FloatType> MixOperandsAndAttrs::operand()
+// DEF-DAG: ::mlir::TypedValue<::mlir::FloatType> MixOperandsAndAttrs::otherArg()
 // DEF-DAG: void MixOperandsAndAttrs::build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::FloatAttr attr, ::mlir::Value operand, ::mlir::FloatAttr otherAttr, ::mlir::Value otherArg)
 // DEF-DAG: ::llvm::APFloat MixOperandsAndAttrs::attr()
 // DEF-DAG: ::llvm::APFloat MixOperandsAndAttrs::otherAttr()
 
 // Test unit attributes.
 // ---
 
 def UnitAttrOp : NS_Op<"unit_attr_op", []> {

mlir/test/mlir-tblgen/op-decl-and-defs.td

 // CHECK-NOT: ::mlir::OpTrait::IsIsolatedFromAbove
 // CHECK: public:
 // CHECK:   using Op::Op;
 // CHECK:   using Adaptor = AOpAdaptor;
 // CHECK:   static constexpr ::llvm::StringLiteral getOperationName() {
 // CHECK:     return ::llvm::StringLiteral("test.a_op");
 // CHECK:   }
 // CHECK:   ::mlir::Operation::operand_range getODSOperands(unsigned index);
-// CHECK:   ::mlir::Value getA();
+// CHECK:   ::mlir::TypedValue<::mlir::IntegerType> getA();
 // CHECK:   ::mlir::Operation::operand_range getB();
 // CHECK:   ::mlir::MutableOperandRange getAMutable();
 // CHECK:   ::mlir::MutableOperandRange getBMutable();
 // CHECK:   ::mlir::Operation::result_range getODSResults(unsigned index);
-// CHECK:   ::mlir::Value getR();
+// CHECK:   ::mlir::TypedValue<::mlir::IntegerType> getR();
 // CHECK:   ::mlir::Region &getSomeRegion();
 // CHECK:   ::mlir::MutableArrayRef<::mlir::Region> getSomeRegions();
 // CHECK:   ::mlir::IntegerAttr getAttr1Attr()
 // CHECK:   uint32_t getAttr1();
 // CHECK:   ::mlir::FloatAttr getAttr2Attr()
 // CHECK:   ::llvm::Optional< ::llvm::APFloat > getAttr2();
 // CHECK:   ::mlir::Attribute removeAttr2Attr();
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Value val);

 def NS_EOp : NS_Op<"op_with_optionals", []> {
   let arguments = (ins Optional<I32>:$a);
   let results = (outs Optional<F32>:$b);
 }
 
 // CHECK-LABEL: NS::EOp declarations
 // CHECK:   ::mlir::Value getA();
 // CHECK:   ::mlir::MutableOperandRange getAMutable();
-// CHECK:   ::mlir::Value getB();
+// CHECK:   ::mlir::TypedValue<::mlir::FloatType> getB();
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, /*optional*/::mlir::Type b, /*optional*/::mlir::Value a)
 
 // Check that all types match constraint results in generating builder.
 // ---
 
 def NS_FOp : NS_Op<"op_with_all_types_constraint",
     [AllTypesMatch<["a", "b"]>]> {
   let arguments = (ins AnyType:$a);

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

 // CHECK-NEXT:    return *getODSOperands(1).begin();
 
 // CHECK-LABEL: ::mlir::ValueRange OpDAdaptor::input3
 // CHECK-NEXT:    return getODSOperands(2);
 
 // CHECK-LABEL: ::mlir::Operation::operand_range OpD::input1
 // CHECK-NEXT: return getODSOperands(0);
 
-// CHECK-LABEL: ::mlir::Value OpD::input2
+// CHECK-LABEL: ::mlir::TypedValue<::mlir::TensorType> OpD::input2
 // CHECK-NEXT: return *getODSOperands(1).begin();
 
 // CHECK-LABEL: OpD::build
 // CHECK-NEXT: odsState.addOperands(input1);
 // CHECK-NEXT: odsState.addOperands(input2);
 // CHECK-NEXT: odsState.addOperands(input3);

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

 
 def OpI : NS_Op<"mix_variadic_and_normal_results_op", [SameVariadicResultSize]> {
   let results = (outs Variadic<AnyTensor>:$output1, AnyTensor:$output2, Variadic<AnyTensor>:$output3);
 }
 
 // CHECK-LABEL: ::mlir::Operation::result_range OpI::output1
 // CHECK-NEXT:    return getODSResults(0);
 
-// CHECK-LABEL: ::mlir::Value OpI::output2
+// CHECK-LABEL: ::mlir::TypedValue<::mlir::TensorType> OpI::output2
 // CHECK-NEXT:    return *getODSResults(1).begin();
 
 // CHECK-LABEL: OpI::build
 // CHECK-NEXT:    odsState.addTypes(output1);
 // CHECK-NEXT:    odsState.addTypes(output2);
 // CHECK-NEXT:    odsState.addTypes(output3);
 
 // Test that if the only operand is variadic, we access the first value in the

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

if (numVariadic == 0) {
     for (auto &it : odsValues)
       isVariadic.push_back(it.isVariableLength() ? "true" : "false");
     std::string isVariadicList = llvm::join(isVariadic, ", ");
     body << formatv(sameVariadicSizeValueRangeCalcCode, isVariadicList,
                     numNonVariadic, numVariadic, rangeSizeCall, "operand");
   }
 }
 
+static std::string generateTypeForGetter(bool isAdaptor,
+                                         const NamedTypeConstraint &value) {
+  std::string str = "::mlir::Value";
+  /// If the CPPClassName is not a fully qualified type. Uses of types
+  /// across Dialect fail because they are not in the correct namespace. So we
+  /// dont generate TypedValue unless the type is fully qualified.
+  /// getCPPClassName doesn't return the fully qualified path for
+  /// `mlir::pdl::OperationType` see
+  /// https://github.com/llvm/llvm-project/issues/57279.
+  /// Adaptor will have values that are not from the type of their operation and
+  /// this is expected, so we dont generate TypedValue for Adaptor
+  if (!isAdaptor && value.constraint.getCPPClassName() != "::mlir::Type" &&
+      StringRef(value.constraint.getCPPClassName()).startswith("::"))
+    str = llvm::formatv("::mlir::TypedValue<{0}>",
+                        value.constraint.getCPPClassName())
+              .str();
+  return str;
+}
+
 // Generates the named operand getter methods for the given Operator `op` and
 // puts them in `opClass`.  Uses `rangeType` as the return type of getters that
 // return a range of operands (individual operands are `Value ` and each
 // element in the range must also be `Value `); use `rangeBeginCall` to get
 // an iterator to the beginning of the operand range; use `rangeSizeCall` to
 // obtain the number of operands. `getOperandCallPattern` contains the code
 // necessary to obtain a single operand whose position will be substituted
 // instead of
static void generateNamedOperandGetters(const Operator &op, Class &opClass,
   // Then we emit nicer named getter methods by redirecting to the "sink" getter
   // method.
   for (int i = 0; i != numOperands; ++i) {
     const auto &operand = op.getOperand(i);
     if (operand.name.empty())
       continue;
     for (StringRef name : op.getGetterNames(operand.name)) {
       if (operand.isOptional()) {
-        m = opClass.addMethod("::mlir::Value", name);
+        m = opClass.addMethod(generateTypeForGetter(isAdaptor, operand), name);
         ERROR_IF_PRUNED(m, name, op);
         m->body() << "  auto operands = getODSOperands(" << i << ");\n"
                   << "  return operands.empty() ? ::mlir::Value() : "
                      "*operands.begin();";
       } else if (operand.isVariadicOfVariadic()) {
         std::string segmentAttr = op.getGetterName(
             operand.constraint.getVariadicOfVariadicSegmentSizeAttr());
         if (isAdaptor) {
for (StringRef name : op.getGetterNames(operand.name)) {
         ERROR_IF_PRUNED(m, name, op);
         m->body() << "  return getODSOperands(" << i << ").split("
                   << segmentAttr << "Attr());";
       } else if (operand.isVariadic()) {
         m = opClass.addMethod(rangeType, name);
         ERROR_IF_PRUNED(m, name, op);
         m->body() << "  return getODSOperands(" << i << ");";
       } else {
-        m = opClass.addMethod("::mlir::Value", name);
+        m = opClass.addMethod(generateTypeForGetter(isAdaptor, operand), name);
         ERROR_IF_PRUNED(m, name, op);
         m->body() << "  return *getODSOperands(" << i << ").begin();";
       }
     }
   }
 }
 
 void OpEmitter::genNamedOperandGetters() {
m->body() << formatv(valueRangeReturnCode, "getOperation()->result_begin()",
                        "getODSResultIndexAndLength(index)");
 
   for (int i = 0; i != numResults; ++i) {
     const auto &result = op.getResult(i);
     if (result.name.empty())
       continue;
     for (StringRef name : op.getGetterNames(result.name)) {
       if (result.isOptional()) {
-        m = opClass.addMethod("::mlir::Value", name);
+        m = opClass.addMethod(
+            generateTypeForGetter(/*isAdaptor=*/false, result), name);
         ERROR_IF_PRUNED(m, name, op);
         m->body()
             << "  auto results = getODSResults(" << i << ");\n"
             << "  return results.empty() ? ::mlir::Value() : *results.begin();";
       } else if (result.isVariadic()) {
         m = opClass.addMethod("::mlir::Operation::result_range", name);
         ERROR_IF_PRUNED(m, name, op);
         m->body() << "  return getODSResults(" << i << ");";
       } else {
-        m = opClass.addMethod("::mlir::Value", name);
+        m = opClass.addMethod(
+            generateTypeForGetter(/*isAdaptor=*/false, result), name);
         ERROR_IF_PRUNED(m, name, op);
         m->body() << "  return *getODSResults(" << i << ").begin();";
       }
     }
   }
 }
 
 void OpEmitter::genNamedRegionGetters() {