[mlir][NFC] Update Operation::getResultTypes to use ArrayRef<Type> instead of iterator_range.

[rriddle]

Summary

The new internal representation of operation results now allows for accessing the result types to be more efficient. Changing the API to ArrayRef is more efficient and removes the need to explicitly materialize vectors in several places.

[merge_guards_bot]

Unit tests: fail. 62150 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[bondhugula]

mlir/include/mlir/IR/Operation.h

266

Nit: result_type_range -> ArrayRef<Type>

result_type_range has only one use.

[bondhugula]

mlir/lib/IR/Operation.cpp

554

result_type_range -> ArrayRef<Type>

mlir/test/lib/TestDialect/TestPatterns.cpp

257

This pattern looks like a concern. Do we get a warning if we didn't use the Type ctor? It's not clear to me why we need it now.

[rriddle]

mlir/include/mlir/IR/Operation.h

266

It is used in other places, e.g. in multi-result operations. I'd rather not remove at it is consistent with the rest of the ranges.

mlir/test/lib/TestDialect/TestPatterns.cpp

257

It is necessary because the methods on Type are non-const and the result type range now returns const Type elements, instead of a Type.

[jpienaar]

mlir/test/lib/TestDialect/TestPatterns.cpp

257

Wouldn't this then result in us having multiple of these casts now? It seems like we'll end up adding more const casts just to query the types.

mlir/include/mlir/IR/OpImplementation.h

 
 template <typename IteratorT>
 inline OpAsmPrinter &
 operator<<(OpAsmPrinter &p,
            const iterator_range<ValueTypeIterator<IteratorT>> &types) {
   interleaveComma(types, p);
   return p;
 }
+inline OpAsmPrinter &operator<<(OpAsmPrinter &p, ArrayRef<Type> types) {
+  interleaveComma(types, p);
+  return p;
+}
 
 //===----------------------------------------------------------------------===//
 // OpAsmParser
 //===----------------------------------------------------------------------===//
 
 /// The OpAsmParser has methods for interacting with the asm parser: parsing
 /// things from it, emitting errors etc.  It has an intentionally high-level API
 /// that is designed to reduce/constrain syntax innovation in individual

mlir/include/mlir/IR/Operation.h

public:
   result_iterator result_end() { return getResults().end(); }
   result_range getResults() { return result_range(this); }
 
   result_range getOpResults() { return getResults(); }
   OpResult getOpResult(unsigned idx) { return getResult(idx); }
 
   /// Support result type iteration.
   using result_type_iterator = result_range::type_iterator;
-  using result_type_range = iterator_range<result_type_iterator>;
-  result_type_iterator result_type_begin() { return result_begin(); }
-  result_type_iterator result_type_end() { return result_end(); }
-  result_type_range getResultTypes() { return getResults().getTypes(); }
+  using result_type_range = ArrayRef<Type>;
+  result_type_iterator result_type_begin() { return getResultTypes().begin(); }
+  result_type_iterator result_type_end() { return getResultTypes().end(); }
+  result_type_range getResultTypes();
 
   //===--------------------------------------------------------------------===//
   // Attributes
   //===--------------------------------------------------------------------===//
 
   // Operations may optionally carry a list of attributes that associate
   // constants to names.  Attributes may be dynamically added and removed over
   // the lifetime of an operation.

mlir/include/mlir/IR/OperationSupport.h

class ResultRange final
     : public indexed_accessor_range<ResultRange, Operation *, OpResult,
                                     OpResult, OpResult> {
 public:
   using indexed_accessor_range<ResultRange, Operation *, OpResult, OpResult,
                                OpResult>::indexed_accessor_range;
   ResultRange(Operation *op);
 
   /// Returns the types of the values within this range.
-  using type_iterator = ValueTypeIterator<iterator>;
-  iterator_range<type_iterator> getTypes() const { return {begin(), end()}; }
+  using type_iterator = ArrayRef<Type>::iterator;
+  ArrayRef<Type> getTypes() const;
 
 private:
   /// See `indexed_accessor_range` for details.
   static OpResult dereference(Operation *op, ptrdiff_t index);
 
   /// Allow access to `dereference_iterator`.
   friend indexed_accessor_range<ResultRange, Operation *, OpResult, OpResult,
                                 OpResult>;

mlir/lib/Analysis/InferTypeOpInterface.cpp

 
 LogicalResult mlir::detail::verifyInferredResultTypes(Operation *op) {
   SmallVector<Type, 4> inferedReturnTypes;
   auto retTypeFn = cast<InferTypeOpInterface>(op);
   if (failed(retTypeFn.inferReturnTypes(op->getContext(), op->getLoc(),
                                         op->getOperands(), op->getAttrs(),
                                         op->getRegions(), inferedReturnTypes)))
     return failure();
-  SmallVector<Type, 4> resultTypes(op->getResultTypes());
-  if (!retTypeFn.isCompatibleReturnTypes(inferedReturnTypes, resultTypes))
+  if (!retTypeFn.isCompatibleReturnTypes(inferedReturnTypes,
+                                         op->getResultTypes()))
     return op->emitOpError(
         "inferred type incompatible with return type of operation");
   return success();
 }

mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp

struct OneToOneLLVMOpLowering : public LLVMLegalizationPattern<SourceOp> {
   // preserve attributes.
   PatternMatchResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
     unsigned numResults = op->getNumResults();
 
     Type packedType;
     if (numResults != 0) {
-      packedType = this->lowering.packFunctionResults(
-          llvm::to_vector<4>(op->getResultTypes()));
+      packedType = this->lowering.packFunctionResults(op->getResultTypes());
       if (!packedType)
         return this->matchFailure();
     }
 
     auto newOp = rewriter.create<TargetOp>(op->getLoc(), packedType, operands,
                                            op->getAttrs());
 
     // If the operation produced 0 or 1 result, return them immediately.

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

if (isDirect)
     p.printSymbolName(callee.getValue());
   else
     p << op.getOperand(0);
 
   p << '(' << op.getOperands().drop_front(isDirect ? 0 : 1) << ')';
   p.printOptionalAttrDict(op.getAttrs(), {"callee"});
 
   // Reconstruct the function MLIR function type from operand and result types.
-  SmallVector<Type, 1> resultTypes(op.getResultTypes());
   SmallVector<Type, 8> argTypes(
       llvm::drop_begin(op.getOperandTypes(), isDirect ? 0 : 1));
 
-  p << " : " << FunctionType::get(argTypes, resultTypes, op.getContext());
+  p << " : "
+    << FunctionType::get(argTypes, op.getResultTypes(), op.getContext());
 }
 
 // <operation> ::= `llvm.call` (function-id | ssa-use) `(` ssa-use-list `)`
 //                 attribute-dict? `:` function-type
 static ParseResult parseCallOp(OpAsmParser &parser, OperationState &result) {
   SmallVector<OpAsmParser::OperandType, 8> operands;
   Type type;
   SymbolRefAttr funcAttr;

mlir/lib/Dialect/SPIRV/SPIRVOps.cpp

static ParseResult parseFunctionCallOp(OpAsmParser &parser,
 
   return failure(parser.addTypesToList(funcType.getResults(), state.types) ||
                  parser.resolveOperands(operands, funcType.getInputs(), loc,
                                         state.operands));
 }
 
 static void print(spirv::FunctionCallOp functionCallOp, OpAsmPrinter &printer) {
   SmallVector<Type, 4> argTypes(functionCallOp.getOperandTypes());
-  SmallVector<Type, 1> resultTypes(functionCallOp.getResultTypes());
-  Type functionType =
-      FunctionType::get(argTypes, resultTypes, functionCallOp.getContext());
+  Type functionType = FunctionType::get(
+      argTypes, functionCallOp.getResultTypes(), functionCallOp.getContext());
 
   printer << spirv::FunctionCallOp::getOperationName() << ' '
           << functionCallOp.getAttr(kCallee) << '('
           << functionCallOp.arguments() << ") : " << functionType;
 }
 
 static LogicalResult verify(spirv::FunctionCallOp functionCallOp) {
   auto fnName = functionCallOp.callee();

mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp

 }
 
 template <>
 LogicalResult
 Serializer::processOp<spirv::FunctionCallOp>(spirv::FunctionCallOp op) {
   auto funcName = op.callee();
   uint32_t resTypeID = 0;
 
-  SmallVector<Type, 1> resultTypes(op.getResultTypes());
-  if (failed(processType(op.getLoc(),
-                         (resultTypes.empty() ? getVoidType() : resultTypes[0]),
-                         resTypeID))) {
+  Type resultTy = op.getNumResults() ? *op.result_type_begin() : getVoidType();
+  if (failed(processType(op.getLoc(), resultTy, resTypeID)))
     return failure();
-  }
 
   auto funcID = getOrCreateFunctionID(funcName);
   auto funcCallID = getNextID();
   SmallVector<uint32_t, 8> operands{resTypeID, funcCallID, funcID};
 
   for (auto value : op.arguments()) {
     auto valueID = getValueID(value);
     assert(valueID && "cannot find a value for spv.FunctionCall");
     operands.push_back(valueID);
   }
 
-  if (!resultTypes.empty()) {
+  if (!resultTy.isa<NoneType>())
     valueIDMap[op.getResult(0)] = funcCallID;
-  }
 
   return encodeInstructionInto(functionBody, spirv::Opcode::OpFunctionCall,
                                operands);
 }
 
 // Pull in auto-generated Serializer::dispatchToAutogenSerialization() and
 // various Serializer::processOp<...>() specializations.
 #define GET_SERIALIZATION_FNS

mlir/lib/Dialect/StandardOps/Ops.cpp

static LogicalResult verify(CallOp op) {
   for (unsigned i = 0, e = fnType.getNumResults(); i != e; ++i)
     if (op.getResult(i).getType() != fnType.getResult(i))
       return op.emitOpError("result type mismatch");
 
   return success();
 }
 
 FunctionType CallOp::getCalleeType() {
-  SmallVector<Type, 4> resultTypes(getResultTypes());
   SmallVector<Type, 8> argTypes(getOperandTypes());
-  return FunctionType::get(argTypes, resultTypes, getContext());
+  return FunctionType::get(argTypes, getResultTypes(), getContext());
 }
 
 //===----------------------------------------------------------------------===//
 // CallIndirectOp
 //===----------------------------------------------------------------------===//
 namespace {
 /// Fold indirect calls that have a constant function as the callee operand.
 struct SimplifyIndirectCallWithKnownCallee
     : public OpRewritePattern<CallIndirectOp> {
   using OpRewritePattern<CallIndirectOp>::OpRewritePattern;
 
   PatternMatchResult matchAndRewrite(CallIndirectOp indirectCall,
                                      PatternRewriter &rewriter) const override {
     // Check that the callee is a constant callee.
     SymbolRefAttr calledFn;
     if (!matchPattern(indirectCall.getCallee(), m_Constant(&calledFn)))
       return matchFailure();
 
     // Replace with a direct call.
-    SmallVector<Type, 8> callResults(indirectCall.getResultTypes());
-    rewriter.replaceOpWithNewOp<CallOp>(indirectCall, calledFn, callResults,
+    rewriter.replaceOpWithNewOp<CallOp>(indirectCall, calledFn,
+                                        indirectCall.getResultTypes(),
                                         indirectCall.getArgOperands());
     return matchSuccess();
   }
 };
 } // end anonymous namespace.
 
 static ParseResult parseCallIndirectOp(OpAsmParser &parser,
                                        OperationState &result) {

mlir/lib/IR/Operation.cpp

 
 /// Return the number of results held by this operation.
 unsigned Operation::getNumResults() {
   if (!resultType)
     return 0;
   return hasSingleResult ? 1 : resultType.cast<TupleType>().size();
 }
 
+auto Operation::getResultTypes() -> result_type_range {
+  if (!resultType)
+    return llvm::None;
+  if (hasSingleResult)
+    return resultType;
+  return resultType.cast<TupleType>().getTypes();
+}
+
 void Operation::setSuccessor(Block *block, unsigned index) {
   assert(index < getNumSuccessors());
   getBlockOperands()[index].set(block);
 }
 
 auto Operation::getNonSuccessorOperands() -> operand_range {
   return getOperands().take_front(hasSuccessors() ? getSuccessorOperandIndex(0)
                                                   : getNumOperands());
for (unsigned succ = 0, e = getNumSuccessors(); succ != e; ++succ) {
       operands.push_back(nullptr);
 
       // Remap the successors operands.
       for (auto operand : getSuccessorOperands(succ))
         operands.push_back(mapper.lookupOrDefault(operand));
     }
   }
 
-  SmallVector<Type, 8> resultTypes(getResultTypes());
   unsigned numRegions = getNumRegions();
   auto *newOp =
-      Operation::create(getLoc(), getName(), resultTypes, operands, attrs,
+      Operation::create(getLoc(), getName(), getResultTypes(), operands, attrs,
                         successors, numRegions, hasResizableOperandsList());
 
   // Remember the mapping of any results.
   for (unsigned i = 0, e = getNumResults(); i != e; ++i)
     mapper.map(getResult(i), newOp->getResult(i));
 
   return newOp;
 }

 
 LogicalResult OpTrait::impl::verifySameOperandsAndResultType(Operation *op) {
   if (failed(verifyAtLeastNOperands(op, 1)) ||
       failed(verifyAtLeastNResults(op, 1)))
     return failure();
 
   auto type = op->getResult(0).getType();
   auto elementType = getElementTypeOrSelf(type);
-  for (auto resultType : llvm::drop_begin(op->getResultTypes(), 1)) {
+  for (auto resultType : op->getResultTypes().drop_front(1)) {
     if (getElementTypeOrSelf(resultType) != elementType ||
         failed(verifyCompatibleShape(resultType, type)))
       return op->emitOpError()
              << "requires the same type for all operands and results";
   }
   for (auto opType : op->getOperandTypes()) {
     if (getElementTypeOrSelf(opType) != elementType ||
         failed(verifyCompatibleShape(opType, type)))

mlir/lib/IR/OperationSupport.cpp

OperandRange::OperandRange(Operation *op)
     : OperandRange(op->getOpOperands().data(), op->getNumOperands()) {}
 
 //===----------------------------------------------------------------------===//
 // ResultRange
 
 ResultRange::ResultRange(Operation *op)
     : ResultRange(op, /*startIndex=*/0, op->getNumResults()) {}
 
+ArrayRef<Type> ResultRange::getTypes() const {
+  return getBase()->getResultTypes();
+}
+
 /// See `indexed_accessor_range` for details.
 OpResult ResultRange::dereference(Operation *op, ptrdiff_t index) {
   return op->getResult(index);
 }
 
 //===----------------------------------------------------------------------===//
 // ValueRange
 

mlir/lib/Transforms/CSE.cpp

 struct SimpleOperationInfo : public llvm::DenseMapInfo<Operation *> {
   static unsigned getHashValue(const Operation *opC) {
     auto *op = const_cast<Operation *>(opC);
     // Hash the operations based upon their:
     //   - Operation Name
     //   - Attributes
     //   - Result Types
     //   - Operands
-    return hash_combine(
-        op->getName(), op->getAttrList().getDictionary(),
-        hash_combine_range(op->result_type_begin(), op->result_type_end()),
-        hash_combine_range(op->operand_begin(), op->operand_end()));
+    return llvm::hash_combine(
+        op->getName(), op->getAttrList().getDictionary(), op->getResultTypes(),
+        llvm::hash_combine_range(op->operand_begin(), op->operand_end()));
   }
   static bool isEqual(const Operation *lhsC, const Operation *rhsC) {
     auto *lhs = const_cast<Operation *>(lhsC);
     auto *rhs = const_cast<Operation *>(rhsC);
     if (lhs == rhs)
       return true;
     if (lhs == getTombstoneKey() || lhs == getEmptyKey() ||
         rhs == getTombstoneKey() || rhs == getEmptyKey())

mlir/test/lib/TestDialect/TestPatterns.cpp

 // Multi-Level Type-Conversion Rewrite Testing
 struct TestChangeProducerTypeI32ToF32 : public ConversionPattern {
   TestChangeProducerTypeI32ToF32(MLIRContext *ctx)
       : ConversionPattern("test.type_producer", 1, ctx) {}
   PatternMatchResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const final {
     // If the type is I32, change the type to F32.
-    if (!(*op->result_type_begin()).isInteger(32))
+    if (!Type(*op->result_type_begin()).isInteger(32))
       return matchFailure();
     rewriter.replaceOpWithNewOp<TestTypeProducerOp>(op, rewriter.getF32Type());
     return matchSuccess();
   }
 };
 struct TestChangeProducerTypeF32ToF64 : public ConversionPattern {
   TestChangeProducerTypeF32ToF64(MLIRContext *ctx)
       : ConversionPattern("test.type_producer", 1, ctx) {}
   PatternMatchResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const final {
     // If the type is F32, change the type to F64.
-    if (!(*op->result_type_begin()).isF32())
+    if (!Type(*op->result_type_begin()).isF32())
       return matchFailure();
     rewriter.replaceOpWithNewOp<TestTypeProducerOp>(op, rewriter.getF64Type());
     return matchSuccess();
   }
 };
 struct TestChangeProducerTypeF32ToInvalid : public ConversionPattern {
   TestChangeProducerTypeF32ToInvalid(MLIRContext *ctx)
       : ConversionPattern("test.type_producer", 10, ctx) {}
assert(std::distance(origOps.begin(), origOps.end()) == 1 &&
            "One operand expected");
     Value origOp = *origOps.begin();
     SmallVector<Value, 2> remappedOperands;
     // Replicate the remapped original operand twice. Note that we don't used
     // the remapped 'operand' since the goal is testing 'getRemappedValue'.
     remappedOperands.push_back(rewriter.getRemappedValue(origOp));
     remappedOperands.push_back(rewriter.getRemappedValue(origOp));
 
-    SmallVector<Type, 1> resultTypes(op.getResultTypes());
-    rewriter.replaceOpWithNewOp<OneVResOneVOperandOp1>(op, resultTypes,
+    rewriter.replaceOpWithNewOp<OneVResOneVOperandOp1>(op, op.getResultTypes(),
                                                        remappedOperands);
     return matchSuccess();
   }
 };
 
 struct TestRemappedValue : public mlir::FunctionPass<TestRemappedValue> {
   void runOnFunction() override {
     mlir::OwningRewritePatternList patterns;