[PATCH] D74211: [mlir] use unpacked memref descriptors at function boundaries

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added a comment.

Thanks Alex! LGTM. As Nicolas mentioned, I think this should help with the aliasing issue. Once it settles down I should be able to retire the bare ptr calling convention if nobody else is interested in it.



================
Comment at: mlir/docs/ConversionToLLVMDialect.md:279
+               %arg3: !llvm.i64,         // Size in dim 0.
+               %arg4: !llvm.i64) {       // Stide in dim 0.
+  // Populate memref descriptor structure.
----------------
Stide -> Stride


================
Comment at: mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h:121
+
+  /// Creates descriptor structs from individual values consituting them.
+  Operation *materializeConversion(PatternRewriter &rewriter, Type type,
----------------
constituting?


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:224
+LLVMTypeConverter::convertUnrankedMemRefSignature(UnrankedMemRefType type) {
+  (void)type;
+  return {getIndexType(), LLVM::LLVMType::getInt8PtrTy(llvmDialect)};
----------------
Remove `type` if it's not needed?


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:286
+
+/// Creates descriptor structs from indiivdual values consituting them.
+Operation *LLVMTypeConverter::materializeConversion(PatternRewriter &rewriter,
----------------
nicolasvasilache wrote:
> typo individual
constituting


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:858
+      arg = wrapperArgsRange[0];
+    }
+
----------------
drop {}


================
Comment at: mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp:906
         continue;
-      attributes.push_back(attr);
+      }
+
----------------
drop {}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74211/new/

https://reviews.llvm.org/D74211