[MLIR] Fix standard -> LLVM conversion to fail for unsupported memref element type.

[jurahul]

Summary

• Move isSupportedMemRefType() to ConvertToLLVMPatterns and check if the memref element type is supported there.

[jurahul]

This addresses the nit in https://reviews.llvm.org/D90803 more generically.

[jurahul]

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp

2438

This is a leftover. Will remove it.

[jurahul]

Remove leftover check.

[bondhugula]

Would a test case be applicable here? I assume this leads to a pattern match failure and the op staying as is in a simple case.

[jurahul]

AFAICT it's not possible today to hit this, so this turns out to be just defensive programming. MemRef element type is either int/float/index/complex or a vector (which itself can only have int or float elements) and all these types seems to be convertible to LLVM types without any failures. As such, I don't think we will run into this issue today, but will guard against future extensions to MemRef types.

mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h

protected:
 
   // This is a strided getElementPtr variant that linearizes subscripts as:
   //   `base_offset + index_0 * stride_0 + ... + index_n * stride_n`.
   Value getStridedElementPtr(Location loc, Type elementTypePtr,
                              Value descriptor, ValueRange indices,
                              ArrayRef<int64_t> strides, int64_t offset,
                              ConversionPatternRewriter &rewriter) const;
 
+  /// Returns if the givem memref type is supported.
+  bool isSupportedMemRefType(MemRefType type) const;
+
   Value getDataPtr(Location loc, MemRefType type, Value memRefDesc,
                    ValueRange indices,
                    ConversionPatternRewriter &rewriter) const;
 
   /// Returns the type of a pointer to an element of the memref.
   Type getElementPtrType(MemRefType type) const;
 
   /// Determines sizes to be used in the memref descriptor.

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp

Value ConvertToLLVMPattern::getDataPtr(
   SmallVector<int64_t, 4> strides;
   auto successStrides = getStridesAndOffset(type, strides, offset);
   assert(succeeded(successStrides) && "unexpected non-strided memref");
   (void)successStrides;
   return getStridedElementPtr(loc, ptrType, memRefDesc, indices, strides,
                               offset, rewriter);
 }
 
+// Check if the MemRefType `type` is supported by the lowering. We currently
+// only support memrefs with identity maps.
+bool ConvertToLLVMPattern::isSupportedMemRefType(MemRefType type) const {
+  if (!typeConverter.convertType(type.getElementType()))
+    return false;
+  return type.getAffineMaps().empty() ||
+         llvm::all_of(type.getAffineMaps(),
+                      [](AffineMap map) { return map.isIdentity(); });
+}
+
 Type ConvertToLLVMPattern::getElementPtrType(MemRefType type) const {
   auto elementType = type.getElementType();
-  auto structElementType = typeConverter.convertType(elementType);
-  return structElementType.cast<LLVM::LLVMType>().getPointerTo(
-      type.getMemorySpace());
+  auto structElementType = unwrap(typeConverter.convertType(elementType));
+  return structElementType.getPointerTo(type.getMemorySpace());
 }
 
 void ConvertToLLVMPattern::getMemRefDescriptorSizes(
     Location loc, MemRefType memRefType, ArrayRef<Value> dynSizes,
     ConversionPatternRewriter &rewriter, SmallVectorImpl<Value> &sizes) const {
   sizes.reserve(memRefType.getRank());
   unsigned i = 0;
   for (int64_t s : memRefType.getShape())
if (op.getValue().isa<SymbolRefAttr>())
           op, "referring to a symbol outside of the current module");
 
     return LLVM::detail::oneToOneRewrite(op,
                                          LLVM::ConstantOp::getOperationName(),
                                          operands, typeConverter, rewriter);
   }
 };
 
-// Check if the MemRefType `type` is supported by the lowering. We currently
-// only support memrefs with identity maps.
-static bool isSupportedMemRefType(MemRefType type) {
-  return type.getAffineMaps().empty() ||
-         llvm::all_of(type.getAffineMaps(),
-                      [](AffineMap map) { return map.isIdentity(); });
-}
-
 /// Lowering for AllocOp and AllocaOp.
 struct AllocLikeOpLowering : public ConvertToLLVMPattern {
   using ConvertToLLVMPattern::createIndexConstant;
   using ConvertToLLVMPattern::getIndexType;
   using ConvertToLLVMPattern::getVoidPtrType;
   using ConvertToLLVMPattern::typeConverter;
 
   explicit AllocLikeOpLowering(StringRef opName, LLVMTypeConverter &converter)
convertGlobalMemrefTypeToLLVM(MemRefType type,
                               LLVMTypeConverter &typeConverter) {
   // LLVM type for a global memref will be a multi-dimension array. For
   // declarations or uninitialized global memrefs, we can potentially flatten
   // this to a 1D array. However, for global_memref's with an initial value,
   // we do not intend to flatten the ElementsAttribute when going from std ->
   // LLVM dialect, so the LLVM type needs to me a multi-dimension array.
   LLVM::LLVMType elementType =
       unwrap(typeConverter.convertType(type.getElementType()));
   LLVM::LLVMType arrayTy = elementType;
   // Shape has the outermost dim at index 0, so need to walk it backwards
   for (int64_t dim : llvm::reverse(type.getShape()))
     arrayTy = LLVM::LLVMType::getArrayTy(arrayTy, dim);
   return arrayTy;
 }
 
 /// GlobalMemrefOp is lowered to a LLVM Global Variable.
 struct GlobalMemrefOpLowering : public ConvertOpToLLVMPattern<GlobalMemrefOp> {

 };
 
 // Common base for load and store operations on MemRefs.  Restricts the match
 // to supported MemRef types.  Provides functionality to emit code accessing a
 // specific element of the underlying data buffer.
 template <typename Derived>
 struct LoadStoreOpLowering : public ConvertOpToLLVMPattern<Derived> {
   using ConvertOpToLLVMPattern<Derived>::ConvertOpToLLVMPattern;
+  using ConvertOpToLLVMPattern<Derived>::isSupportedMemRefType;
   using Base = LoadStoreOpLowering<Derived>;
 
   LogicalResult match(Operation *op) const override {
     MemRefType type = cast<Derived>(op).getMemRefType();
     return isSupportedMemRefType(type) ? success() : failure();
   }
 };