[mlir] Add an assertion on creating an Operation with null result types

[ftynse]

Summary

Null types are commonly used as an error marker. Catch them in the constructor
of Operation if they are present in the result type list, as otherwise this
could lead to further surprising behavior when querying op result types.

Fix AsyncToLLVM and StandardToLLVM that were using null types when constructing
operations.

[ftynse]

mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp

226

We could also consider adding a TypeRange constructor that creates an empty range out of a null type, but that would look surprising to me in the case of

Type t = getTypeOrNull();
something(TypeRange(t)); // empty range not really expected

[rriddle]

Thanks for the quick fix!

mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp

226

I was actually thinking the opposite, i.e. assert that all provided types(from an ArrayRef) to a TypeRange are non-null.

[ftynse]

Add a check in TypeRange

mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp

226

Added this, too.

mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp

static void addResumeFunction(ModuleOp module) {
 
   auto resumeOp = moduleBuilder.create<LLVM::LLVMFuncOp>(
       loc, kResume, LLVM::LLVMType::getFunctionTy(voidTy, {i8Ptr}, false));
   resumeOp.setPrivate();
 
   auto *block = resumeOp.addEntryBlock();
   OpBuilder blockBuilder = OpBuilder::atBlockEnd(block);
 
-  blockBuilder.create<LLVM::CallOp>(loc, Type(),
+  blockBuilder.create<LLVM::CallOp>(loc, TypeRange(),
                                     blockBuilder.getSymbolRefAttr(kCoroResume),
                                     resumeOp.getArgument(0));
 
   blockBuilder.create<LLVM::ReturnOp>(loc, ValueRange());
 }
 
 //===----------------------------------------------------------------------===//
 // async.execute op outlining to the coroutine functions.
static CoroMachinery setupCoroMachinery(FuncOp func) {
   builder.setInsertionPointToStart(cleanupBlock);
 
   // Get a pointer to the coroutine frame memory: @llvm.coro.free.
   auto coroMem = builder.create<LLVM::CallOp>(
       loc, i8Ptr, builder.getSymbolRefAttr(kCoroFree),
       ValueRange({coroId.getResult(0), coroHdl.getResult(0)}));
 
   // Free the memory.
-  builder.create<LLVM::CallOp>(loc, Type(), builder.getSymbolRefAttr(kFree),
+  builder.create<LLVM::CallOp>(loc, TypeRange(),
+                               builder.getSymbolRefAttr(kFree),
                                ValueRange(coroMem.getResult(0)));
   // Branch into the suspend block.
   builder.create<BranchOp>(loc, suspendBlock);
 
   // ------------------------------------------------------------------------ //
   // Coroutine suspend block: mark the end of a coroutine and return allocated
   // async token.
   // ------------------------------------------------------------------------ //
outlineExecuteOp(SymbolTable &symbolTable, ExecuteOp execute) {
 
   // Save the coroutine state: @llvm.coro.save
   auto coroSave = builder.create<LLVM::CallOp>(
       loc, LLVM::LLVMTokenType::get(ctx), builder.getSymbolRefAttr(kCoroSave),
       ValueRange({coro.coroHandle}));
 
   // Call async runtime API to execute a coroutine in the managed thread.
   SmallVector<Value, 2> executeArgs = {coro.coroHandle, resumePtr.res()};
-  builder.create<CallOp>(loc, Type(), kExecute, executeArgs);
+  builder.create<CallOp>(loc, TypeRange(), kExecute, executeArgs);
 
   // Split the entry block before the terminator.
   Block *resume = addSuspensionPoint(coro, coroSave.getResult(0),
                                      entryBlock->getTerminator());
 
   // Await on all dependencies before starting to execute the body region.
   builder.setInsertionPointToStart(resume);
   for (size_t i = 0; i < execute.dependencies().size(); ++i)
     builder.create<AwaitOp>(loc, func.getArgument(i));
 
   // Map from function inputs defined above the execute op to the function
   // arguments.
   BlockAndValueMapping valueMapping;
   valueMapping.map(functionInputs, func.getArguments());
 
   // Clone all operations from the execute operation body into the outlined
   // function body, and replace all `async.yield` operations with a call
   // to async runtime to emplace the result token.
   for (Operation &op : execute.body().getOps()) {
     if (isa<async::YieldOp>(op)) {
-      builder.create<CallOp>(loc, kEmplaceToken, Type(), coro.asyncToken);
+      builder.create<CallOp>(loc, kEmplaceToken, TypeRange(), coro.asyncToken);
       continue;
     }
     builder.clone(op, valueMapping);
   }
 
   // Replace the original `async.execute` with a call to outlined function.
   OpBuilder callBuilder(execute);
   auto callOutlinedFunc =
matchAndRewrite(Operation *op, ArrayRef<Value> operands,
     auto outlined = outlinedFunctions.find(func);
     const bool isInCoroutine = outlined != outlinedFunctions.end();
 
     Location loc = op->getLoc();
 
     // Inside regular function we convert await operation to the blocking
     // async API await function call.
     if (!isInCoroutine)
-      rewriter.create<CallOp>(loc, Type(), blockingAwaitFuncName,
+      rewriter.create<CallOp>(loc, TypeRange(), blockingAwaitFuncName,
                               ValueRange(op->getOperand(0)));
 
     // Inside the coroutine we convert await operation into coroutine suspension
     // point, and resume execution asynchronously.
     if (isInCoroutine) {
       const CoroMachinery &coro = outlined->getSecond();
 
       OpBuilder builder(op);
       MLIRContext *ctx = op->getContext();
 
       // A pointer to coroutine resume intrinsic wrapper.
       auto resumeFnTy = AsyncAPI::resumeFunctionType(ctx);
       auto resumePtr = builder.create<LLVM::AddressOfOp>(
           loc, resumeFnTy.getPointerTo(), kResume);
 
       // Save the coroutine state: @llvm.coro.save
       auto coroSave = builder.create<LLVM::CallOp>(
           loc, LLVM::LLVMTokenType::get(ctx),
           builder.getSymbolRefAttr(kCoroSave), ValueRange(coro.coroHandle));
 
       // Call async runtime API to resume a coroutine in the managed thread when
       // the async await argument becomes ready.
       SmallVector<Value, 3> awaitAndExecuteArgs = {
           await.getOperand(), coro.coroHandle, resumePtr.res()};
-      builder.create<CallOp>(loc, Type(), coroAwaitFuncName,
+      builder.create<CallOp>(loc, TypeRange(), coroAwaitFuncName,
                              awaitAndExecuteArgs);
 
       // Split the entry block before the await operation.
       addSuspensionPoint(coro, coroSave.getResult(0), op);
     }
 
     // Original operation was replaced by function call or suspension point.
     rewriter.eraseOp(op);

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp

struct CallOpInterfaceLowering : public ConvertOpToLLVMPattern<CallOpType> {
 
   LogicalResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
     typename CallOpType::Adaptor transformed(operands);
     auto callOp = cast<CallOpType>(op);
 
     // Pack the result types into a struct.
-    Type packedResult;
+    Type packedResult = nullptr;
     unsigned numResults = callOp.getNumResults();
     auto resultTypes = llvm::to_vector<4>(callOp.getResultTypes());
 
     if (numResults != 0) {
       if (!(packedResult =
                 this->typeConverter.packFunctionResults(resultTypes)))
         return failure();
     }
 
     auto promoted = this->typeConverter.promoteOperands(
         op->getLoc(), /*opOperands=*/op->getOperands(), operands, rewriter);
-    auto newOp = rewriter.create<LLVM::CallOp>(op->getLoc(), packedResult,
-                                               promoted, op->getAttrs());
+    auto newOp = rewriter.create<LLVM::CallOp>(
+        op->getLoc(), packedResult ? TypeRange(packedResult) : TypeRange(),
 
     SmallVector<Value, 4> results;
     if (numResults < 2) {
       // If < 2 results, packing did not do anything and we can just return.
       results.append(newOp.result_begin(), newOp.result_end());
     } else {
       // Otherwise, it had been converted to an operation producing a structure.
       // Extract individual results from the structure and return them as list.

mlir/lib/IR/Operation.cpp

 Operation::Operation(Location location, OperationName name,
                      TypeRange resultTypes, unsigned numSuccessors,
                      unsigned numRegions,
                      const MutableDictionaryAttr &attributes,
                      bool hasOperandStorage)
     : location(location), numSuccs(numSuccessors), numRegions(numRegions),
       hasOperandStorage(hasOperandStorage), hasSingleResult(false), name(name),
       attrs(attributes) {
+  assert(llvm::all_of(resultTypes, [](Type t) { return t; }) &&
+         "unexpected null result type");
   if (!resultTypes.empty()) {
     // If there is a single result it is stored in-place, otherwise use a tuple.
     hasSingleResult = resultTypes.size() == 1;
     if (hasSingleResult)
       resultType = resultTypes.front();
     else
       resultType = TupleType::get(resultTypes, location->getContext());
   }

mlir/lib/IR/TypeRange.cpp

 //===- TypeRange.cpp ------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/IR/TypeRange.h"
 #include "mlir/IR/Operation.h"
 using namespace mlir;
 
 //===----------------------------------------------------------------------===//
 // TypeRange
 
 TypeRange::TypeRange(ArrayRef<Type> types)
-    : TypeRange(types.data(), types.size()) {}
+    : TypeRange(types.data(), types.size()) {
+  assert(llvm::all_of(types, [](Type t) { return t; }) &&
+         "attempting to construct a TypeRange with null types");
+}
 TypeRange::TypeRange(OperandRange values)
     : TypeRange(values.begin().getBase(), values.size()) {}
 TypeRange::TypeRange(ResultRange values)
     : TypeRange(values.getBase()->getResultTypes().slice(values.getStartIndex(),
                                                          values.size())) {}
 TypeRange::TypeRange(ArrayRef<Value> values)
     : TypeRange(values.data(), values.size()) {}
 TypeRange::TypeRange(ValueRange values) : TypeRange(OwnerT(), values.size()) {

mlir/test/Conversion/StandardToLLVM/standard-to-llvm.mlir

func @get_gv3_memref() {
   // CHECK: llvm.insertvalue %[[DEADBEEFPTR]], {{.*}}[0] : !llvm.struct<(ptr<float>, ptr<float>, i64)>
   // CHECK: llvm.insertvalue %[[GEP]], {{.*}}[1] : !llvm.struct<(ptr<float>, ptr<float>, i64)>
   // CHECK: %[[OFFSET:.*]] = llvm.mlir.constant(0 : index) : !llvm.i64
   // CHECK: llvm.insertvalue %[[OFFSET]], {{.*}}[2] : !llvm.struct<(ptr<float>, ptr<float>, i64)>
   %0 = get_global_memref @gv3 : memref<f32>
   return
 }
 
+// This should not trigger an assertion by creating an LLVM::CallOp with a
+// nullptr result type.
+
+// CHECK-LABEL: @call_zero_result_func
+func @call_zero_result_func() {
+  // CHECK: call @zero_result_func
+  call @zero_result_func() : () -> ()
+  return
+}
+func private @zero_result_func()