[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `CopyOp` #151206

Jaddyen · 2025-07-29T18:20:26Z

This patch lowers memref.copy to emitc.call_opaque "memcpy".
From:

func.func @copying(%arg0 : memref<9x4x5x7xf32>, %arg1 : memref<9x4x5x7xf32>) {
  memref.copy %arg0, %arg1 : memref<9x4x5x7xf32> to memref<9x4x5x7xf32>
  return
}

To:

#include <cstring>
void copying(float v1[9][4][5][7], float v2[9][4][5][7]) {
  size_t v3 = 0;
  float* v4 = &v2[v3][v3][v3][v3];
  float* v5 = &v1[v3][v3][v3][v3];
  size_t v6 = sizeof(float);
  size_t v7 = 1260;
  size_t v8 = v6 * v7;
  memcpy(v5, v4, v8);
  return;
}

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

llvmbot · 2025-07-29T18:24:08Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-emitc

Author: Jaden Angella (Jaddyen)

Changes

This patch lowers memref.copy to emitc.call_opaque "memcpy".
From:

func.func @<!-- -->copying(%arg0 : memref&lt;9x4xf32&gt;) {
  memref.copy %arg0, %arg0 : memref&lt;9x4xf32&gt; to memref&lt;9x4xf32&gt;
  return
}

To:

void copying(float v1[9][4]) {
  size_t v2 = 0;
  size_t v3 = 0;
  float* v4 = &amp;v1[v2][v3];
  size_t v5 = 0;
  size_t v6 = 0;
  float* v7 = &amp;v1[v5][v6];
  size_t v8 = sizeof(float);
  size_t v9 = 36;
  size_t v10 = v8 * v9;
  memcpy(v7, v4, v10);
  return;
}

Full diff: https://github.com/llvm/llvm-project/pull/151206.diff

5 Files Affected:

(modified) mlir/include/mlir/Conversion/MemRefToEmitC/MemRefToEmitC.h (+2)
(modified) mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp (+75-2)
(modified) mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp (+20-7)
(added) mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir (+24)
(modified) mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir (-8)

diff --git a/mlir/include/mlir/Conversion/MemRefToEmitC/MemRefToEmitC.h b/mlir/include/mlir/Conversion/MemRefToEmitC/MemRefToEmitC.h
index b595b6a308bea..4ea6649d64a92 100644
--- a/mlir/include/mlir/Conversion/MemRefToEmitC/MemRefToEmitC.h
+++ b/mlir/include/mlir/Conversion/MemRefToEmitC/MemRefToEmitC.h
@@ -10,8 +10,10 @@
 
 constexpr const char *alignedAllocFunctionName = "aligned_alloc";
 constexpr const char *mallocFunctionName = "malloc";
+constexpr const char *memcpyFunctionName = "memcpy";
 constexpr const char *cppStandardLibraryHeader = "cstdlib";
 constexpr const char *cStandardLibraryHeader = "stdlib.h";
+constexpr const char *stringLibraryHeader = "string.h";
 
 namespace mlir {
 class DialectRegistry;
diff --git a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
index 6bd0e2d4d4b08..adb0eb77fdf35 100644
--- a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
+++ b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
@@ -97,6 +97,29 @@ Type convertMemRefType(MemRefType opTy, const TypeConverter *typeConverter) {
   return resultTy;
 }
 
+Value calculateMemrefTotalSizeBytes(Location loc, MemRefType memrefType,
+                                    ConversionPatternRewriter &rewriter) {
+  emitc::CallOpaqueOp elementSize = rewriter.create<emitc::CallOpaqueOp>(
+      loc, emitc::SizeTType::get(rewriter.getContext()),
+      rewriter.getStringAttr("sizeof"), ValueRange{},
+      ArrayAttr::get(rewriter.getContext(),
+                     {TypeAttr::get(memrefType.getElementType())}));
+
+  IndexType indexType = rewriter.getIndexType();
+  int64_t numElements = 1;
+  for (int64_t dimSize : memrefType.getShape()) {
+    numElements *= dimSize;
+  }
+  emitc::ConstantOp numElementsValue = rewriter.create<emitc::ConstantOp>(
+      loc, indexType, rewriter.getIndexAttr(numElements));
+
+  Type sizeTType = emitc::SizeTType::get(rewriter.getContext());
+  emitc::MulOp totalSizeBytes = rewriter.create<emitc::MulOp>(
+      loc, sizeTType, elementSize.getResult(0), numElementsValue);
+
+  return totalSizeBytes.getResult();
+}
+
 struct ConvertAlloc final : public OpConversionPattern<memref::AllocOp> {
   using OpConversionPattern::OpConversionPattern;
   LogicalResult
@@ -159,6 +182,55 @@ struct ConvertAlloc final : public OpConversionPattern<memref::AllocOp> {
   }
 };
 
+struct ConvertCopy final : public OpConversionPattern<memref::CopyOp> {
+  using OpConversionPattern::OpConversionPattern;
+
+  LogicalResult
+  matchAndRewrite(memref::CopyOp copyOp, OpAdaptor operands,
+                  ConversionPatternRewriter &rewriter) const override {
+    Location loc = copyOp.getLoc();
+    MemRefType srcMemrefType =
+        dyn_cast<MemRefType>(copyOp.getSource().getType());
+    MemRefType targetMemrefType =
+        dyn_cast<MemRefType>(copyOp.getTarget().getType());
+
+    if (!isMemRefTypeLegalForEmitC(srcMemrefType) ||
+        !isMemRefTypeLegalForEmitC(targetMemrefType)) {
+      return rewriter.notifyMatchFailure(
+          loc, "incompatible memref type for EmitC conversion");
+    }
+
+    emitc::ConstantOp zeroIndex = rewriter.create<emitc::ConstantOp>(
+        loc, rewriter.getIndexType(), rewriter.getIndexAttr(0));
+    auto srcArrayValue =
+        dyn_cast<TypedValue<emitc::ArrayType>>(operands.getSource());
+
+    emitc::SubscriptOp srcSubPtr = rewriter.create<emitc::SubscriptOp>(
+        loc, srcArrayValue, ValueRange{zeroIndex, zeroIndex});
+    emitc::ApplyOp srcPtr = rewriter.create<emitc::ApplyOp>(
+        loc, emitc::PointerType::get(srcMemrefType.getElementType()),
+        rewriter.getStringAttr("&"), srcSubPtr);
+
+    auto arrayValue =
+        dyn_cast<TypedValue<emitc::ArrayType>>(operands.getTarget());
+    emitc::SubscriptOp targetSubPtr = rewriter.create<emitc::SubscriptOp>(
+        loc, arrayValue, ValueRange{zeroIndex, zeroIndex});
+    emitc::ApplyOp targetPtr = rewriter.create<emitc::ApplyOp>(
+        loc, emitc::PointerType::get(targetMemrefType.getElementType()),
+        rewriter.getStringAttr("&"), targetSubPtr);
+
+    emitc::CallOpaqueOp memCpyCall = rewriter.create<emitc::CallOpaqueOp>(
+        loc, TypeRange{}, "memcpy",
+        ValueRange{
+            targetPtr.getResult(), srcPtr.getResult(),
+            calculateMemrefTotalSizeBytes(loc, srcMemrefType, rewriter)});
+
+    rewriter.replaceOp(copyOp, memCpyCall.getResults());
+
+    return success();
+  }
+};
+
 struct ConvertGlobal final : public OpConversionPattern<memref::GlobalOp> {
   using OpConversionPattern::OpConversionPattern;
 
@@ -320,6 +392,7 @@ void mlir::populateMemRefToEmitCTypeConversion(TypeConverter &typeConverter) {
 
 void mlir::populateMemRefToEmitCConversionPatterns(
     RewritePatternSet &patterns, const TypeConverter &converter) {
-  patterns.add<ConvertAlloca, ConvertAlloc, ConvertGlobal, ConvertGetGlobal,
-               ConvertLoad, ConvertStore>(converter, patterns.getContext());
+  patterns.add<ConvertAlloca, ConvertAlloc, ConvertCopy, ConvertGlobal,
+               ConvertGetGlobal, ConvertLoad, ConvertStore>(
+      converter, patterns.getContext());
 }
diff --git a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp
index e78dd76d6e256..8e965b42f1043 100644
--- a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp
+++ b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp
@@ -18,6 +18,7 @@
 #include "mlir/IR/Attributes.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Transforms/DialectConversion.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace mlir {
 #define GEN_PASS_DEF_CONVERTMEMREFTOEMITC
@@ -27,6 +28,15 @@ namespace mlir {
 using namespace mlir;
 
 namespace {
+
+emitc::IncludeOp addHeader(OpBuilder &builder, ModuleOp module,
+                           StringRef headerName) {
+  StringAttr includeAttr = builder.getStringAttr(headerName);
+  return builder.create<emitc::IncludeOp>(
+      module.getLoc(), includeAttr,
+      /*is_standard_include=*/builder.getUnitAttr());
+}
+
 struct ConvertMemRefToEmitCPass
     : public impl::ConvertMemRefToEmitCBase<ConvertMemRefToEmitCPass> {
   using Base::Base;
@@ -57,7 +67,8 @@ struct ConvertMemRefToEmitCPass
     mlir::ModuleOp module = getOperation();
     module.walk([&](mlir::emitc::CallOpaqueOp callOp) {
       if (callOp.getCallee() != alignedAllocFunctionName &&
-          callOp.getCallee() != mallocFunctionName) {
+          callOp.getCallee() != mallocFunctionName &&
+          callOp.getCallee() != memcpyFunctionName) {
         return mlir::WalkResult::advance();
       }
 
@@ -76,12 +87,14 @@ struct ConvertMemRefToEmitCPass
       }
 
       mlir::OpBuilder builder(module.getBody(), module.getBody()->begin());
-      StringAttr includeAttr =
-          builder.getStringAttr(options.lowerToCpp ? cppStandardLibraryHeader
-                                                   : cStandardLibraryHeader);
-      builder.create<mlir::emitc::IncludeOp>(
-          module.getLoc(), includeAttr,
-          /*is_standard_include=*/builder.getUnitAttr());
+      StringRef headerName;
+      if (callOp.getCallee() == memcpyFunctionName)
+        headerName = stringLibraryHeader;
+      else
+        headerName = options.lowerToCpp ? cppStandardLibraryHeader
+                                        : cStandardLibraryHeader;
+
+      addHeader(builder, module, headerName);
       return mlir::WalkResult::interrupt();
     });
   }
diff --git a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir
new file mode 100644
index 0000000000000..4b6eb50807513
--- /dev/null
+++ b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir
@@ -0,0 +1,24 @@
+// RUN: mlir-opt -convert-memref-to-emitc %s  | FileCheck %s
+
+func.func @copying(%arg0 : memref<2x4xf32>) {
+  memref.copy %arg0, %arg0 : memref<2x4xf32> to memref<2x4xf32>
+  return
+}
+
+// CHECK: module {
+// CHECK-NEXT:  emitc.include <"string.h">
+// CHECK-LABEL:  copying
+// CHECK-NEXT: %0 = builtin.unrealized_conversion_cast %arg0 : memref<2x4xf32> to !emitc.array<2x4xf32>
+// CHECK-NEXT: %1 = "emitc.constant"() <{value = 0 : index}> : () -> index
+// CHECK-NEXT: %2 = emitc.subscript %0[%1, %1] : (!emitc.array<2x4xf32>, index, index) -> !emitc.lvalue<f32>
+// CHECK-NEXT: %3 = emitc.apply "&"(%2) : (!emitc.lvalue<f32>) -> !emitc.ptr<f32>
+// CHECK-NEXT: %4 = emitc.subscript %0[%1, %1] : (!emitc.array<2x4xf32>, index, index) -> !emitc.lvalue<f32>
+// CHECK-NEXT: %5 = emitc.apply "&"(%4) : (!emitc.lvalue<f32>) -> !emitc.ptr<f32>
+// CHECK-NEXT: %6 = emitc.call_opaque "sizeof"() {args = [f32]} : () -> !emitc.size_t
+// CHECK-NEXT: %7 = "emitc.constant"() <{value = 8 : index}> : () -> index
+// CHECK-NEXT: %8 = emitc.mul %6, %7 : (!emitc.size_t, index) -> !emitc.size_t
+// CHECK-NEXT: emitc.call_opaque "memcpy"(%5, %3, %8) : (!emitc.ptr<f32>, !emitc.ptr<f32>, !emitc.size_t) -> ()
+// CHECK-NEXT:    return
+// CHECK-NEXT:  }
+// CHECK-NEXT:}
+
diff --git a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
index fda01974d3fc8..b6eccfc8f0050 100644
--- a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
+++ b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
@@ -1,13 +1,5 @@
 // RUN: mlir-opt -convert-memref-to-emitc %s -split-input-file -verify-diagnostics
 
-func.func @memref_op(%arg0 : memref<2x4xf32>) {
-  // expected-error@+1 {{failed to legalize operation 'memref.copy'}}
-  memref.copy %arg0, %arg0 : memref<2x4xf32> to memref<2x4xf32>
-  return
-}
-
-// -----
-
 func.func @alloca_with_dynamic_shape() {
   %0 = index.constant 1
   // expected-error@+1 {{failed to legalize operation 'memref.alloca'}}

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

aniragil · 2025-07-31T13:36:57Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Can take the more generic OpBuilder here instead of a Rewriter?

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

aniragil · 2025-07-31T13:46:47Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

I'd split this to two checks to specify in the message which memref was incompatible.

aniragil · 2025-07-31T13:56:24Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

This only works for 2D arrays

aniragil · 2025-07-31T13:57:47Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Duplicates the work for srcArrayValue, can be folded into a lambda.

aniragil · 2025-07-31T14:13:25Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Use cast (these arguments must be MemRefs by op definition).

aniragil · 2025-07-31T14:16:02Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

This function should be probably be named addStandardHeader as it always sets is_standard_include.

aniragil · 2025-07-31T14:35:02Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

Why not follow the C/C++ distinction as with stdlib, i.e. cstring vs string.h?

ilovepi · 2025-08-04T18:28:30Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

There should probably be a c++ test too.

you only have a test for <string.h>, right? in the c++ mode, you'd have , so you probably need a test for the cpp mode too. Sorry for not being more explicit.

ack, thanks for the feedback!

aniragil · 2025-08-05T06:29:01Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

You're already capturing rewriter and zeroIndex, no need to pass them as arguments (if you move zeroIndex creation above the lambda).

Usually better to capture exactly lambda uses than capture everything (&).

ack, thanks for the feedback!

aniragil · 2025-08-05T06:37:05Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Why not take element type from arrayValue (as done for the rank)? Would save passing memrefType as arg.

ack, thanks for the feedback!

aniragil · 2025-08-05T11:43:42Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Use cast rather than dyn_cast when you expect cast to succeed (it will assert on failure).

aniragil · 2025-08-05T11:43:52Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Same as above.

aniragil · 2025-08-05T11:46:52Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Rewriter is a builder, you can just pass it as argument (and should, as you're working within a conversion pattern).

aniragil · 2025-08-05T12:06:31Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

Change itself is good (removing braces from single-line blocks) but should be done on a separate PR to avoid cluttering this one with unrelated modifications.

ideally, yes.
but im already modifying this portion of code and this would be a single line change.

ideally, yes. but im already modifying this portion of code and this would be a single line change.

Practically too. It's not about the number of lines or their proximity to other changes. LLVM's contribution policy requires patches to be minimal. More specifically:

* not contain any unrelated changes * be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.

ack, thanks for the feedback!

aniragil · 2025-08-05T12:07:37Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

Same here.
Also, shouldn't the code here also check for c/cppStringLibraryHeader?

yes, i should!
thanks for the pointer!

aniragil · 2025-08-06T06:18:26Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

ideally, yes. but im already modifying this portion of code and this would be a single line change.

Practically too. It's not about the number of lines or their proximity to other changes. LLVM's contribution policy requires patches to be minimal. More specifically:

* not contain any unrelated changes * be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.

aniragil · 2025-08-06T09:03:29Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

Not sure this works as expected: the same module may contain both malloc and memcpy calls. The code seems to settle for a single call that needs an include directive, and IINM not necessarily the right one (as isExpectedIncludeOp doesn't compare includeOp against the specific callOp.getCallee()). Am I missing something? (In any case please add such a test).

ack, thanks for the feedback!

ilovepi · 2025-08-06T17:14:24Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

Most of the lines are common between the two prefixes, and can/should stay CHECK lines, since those are always checked. The lines that differ can have their own prefixes. You can remove the -NEXT suffix as needed to make the test pass, but I'd only expect you to need to change at most one line. So from a quick skim it seems like you'll only need to change 3 lines from the old version to make the addition check work.

ack, thanks for the feedback!

ilovepi · 2025-08-07T15:36:12Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

Suggested change

// CPP-LABEL: copying

// CHECK-LABEL: copying

Wasn't this part of the output in both groups in the previous version?

it was!
thanks for the feedback.

ilovepi · 2025-08-07T15:36:45Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

Suggested change

// NOCPP: module {

// CHECK: module {

This is common between both.

ilovepi · 2025-08-07T15:37:06Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

Suggested change

// NOCPP-NEXT: emitc.include <"string.h">

// NOCPP: emitc.include <"string.h">

ilovepi · 2025-08-07T15:37:34Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

Suggested change

// CPP: module {

// CPP-NEXT: emitc.include <"cstring">

// CPP: emitc.include <"cstring">

ilovepi · 2025-08-07T15:39:30Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-alloc-copy.mlir

Suggested change

// NOCPP: module {

// NOCPP-NEXT: emitc.include <"string.h">

// NOCPP-NEXT: emitc.include <"stdlib.h">

// CPP: module {

// CPP-NEXT: emitc.include <"cstring">

// CPP-NEXT: emitc.include <"cstdlib">

// CHECK: module {

// NOCPP: emitc.include <"string.h">

// NOCPP-NEXT: emitc.include <"stdlib.h">

// CPP: emitc.include <"cstring">

// CPP-NEXT: emitc.include <"cstdlib">

ilovepi · 2025-08-07T15:53:22Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

maybe?

Suggested change

int64_t numElements = 1;

for (int64_t dimSize : memrefType.getShape()) {

numElements *= dimSize;

}

int64_t numElements = std::accumulate(memrefType.getShape().begin(), memrefType.getShape().end(), int64_t{1}, std::multiplies<int64_t>());

ilovepi · 2025-08-07T16:07:35Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

You can use the SmallVector (size_t Size, const T &Value) constructor to avoid the loop and subsequent push_back()s. If nothing else, its a good practice to reserve(rank) elements before the loop starts, and avoid potential allocations.

ilovepi · 2025-08-07T16:13:35Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Suggested change

Value calculateMemrefTotalSizeBytes(Location loc, MemRefType memrefType,

static Value calculateMemrefTotalSizeBytes(Location loc, MemRefType memrefType,

I don't think this gets used outside this TU, right?

aniragil · 2025-08-07T12:37:20Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

Simpler to push_back directly on each case.

aniragil · 2025-08-07T12:41:03Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

Modules may have multiple calls to malloc/memcpy. Use a set instead? (IINM this currently doesn't cause double header inclusion because of the repeated scan below, but worth adding such a test case).

aniragil · 2025-08-07T19:08:20Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitCPass.cpp

This scans all module-level ops for each expected header. Can instead scan once in advance to find all available headers, then when scanning call_opaques can skip those whose requested header is already included.

ilovepi

LGTM

ilovepi · 2025-08-08T17:14:36Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir

+// NOCPP:  emitc.include <"string.h">
+
+// CPP:  emitc.include <"cstring">


Suggested change

// NOCPP: emitc.include <"string.h">

// CPP: emitc.include <"cstring">

// NOCPP: emitc.include <"string.h">

// CPP: emitc.include <"cstring">

I think this a more readable formatting, since its makes it a bit obvious that these are variants of the same line. This is a rather minor nit though, so up to you.

i've addressed in new changes.

Jaddyen · 2025-08-08T21:28:02Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+  return totalSizeBytes.getResult();
+}
+
+static emitc::ApplyOp


I'm making this a function instead a lambda so that we can use it later when lowering other memref ops like extract_strided_metadata and reinterpret_cast that need a pointer to the first element of the array.

aniragil

One more nit

aniragil · 2025-08-09T09:05:15Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+  int64_t rank = arrayValue.getType().getRank();
+  llvm::SmallVector<mlir::Value> indices(rank, zeroIndex);
+
+  emitc::SubscriptOp subPtr =
+      builder.create<emitc::SubscriptOp>(loc, arrayValue, ValueRange(indices));
+  emitc::ApplyOp ptr = builder.create<emitc::ApplyOp>(
+      loc, emitc::PointerType::get(arrayValue.getType().getElementType()),
+      builder.getStringAttr("&"), subPtr);


Suggested change

int64_t rank = arrayValue.getType().getRank();

llvm::SmallVector<mlir::Value> indices(rank, zeroIndex);

emitc::SubscriptOp subPtr =

builder.create<emitc::SubscriptOp>(loc, arrayValue, ValueRange(indices));

emitc::ApplyOp ptr = builder.create<emitc::ApplyOp>(

loc, emitc::PointerType::get(arrayValue.getType().getElementType()),

builder.getStringAttr("&"), subPtr);

ArrayType arrayType = arrayValue.getType();

llvm::SmallVector<mlir::Value> indices(arrayType.getRank(), zeroIndex);

emitc::SubscriptOp subPtr =

builder.create<emitc::SubscriptOp>(loc, arrayValue, ValueRange(indices));

emitc::ApplyOp ptr = builder.create<emitc::ApplyOp>(

loc, emitc::PointerType::get(arrayType.getElementType()),

builder.getStringAttr("&"), subPtr);

aniragil

LGTM, thanks!

Jaddyen commented Jul 29, 2025

View reviewed changes

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp Outdated Show resolved Hide resolved

Jaddyen marked this pull request as ready for review July 29, 2025 18:23

Jaddyen requested review from aniragil, mgehre-amd and simon-camp July 29, 2025 18:23

llvmbot added mlir mlir:emitc labels Jul 29, 2025

Jaddyen requested review from ilovepi, jpienaar and mtrofin July 29, 2025 18:24

ilovepi reviewed Jul 29, 2025

View reviewed changes

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-copy.mlir Outdated Show resolved Hide resolved

aniragil requested changes Jul 31, 2025

View reviewed changes

ilovepi reviewed Aug 4, 2025

View reviewed changes

Jaddyen requested a review from aniragil August 4, 2025 22:05

aniragil requested changes Aug 5, 2025

View reviewed changes

Jaddyen requested review from aniragil and ilovepi August 5, 2025 17:45

Jaddyen mentioned this pull request Aug 5, 2025

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering extract_strided_metadata #152208

Draft

aniragil requested changes Aug 6, 2025

View reviewed changes

aniragil mentioned this pull request Aug 6, 2025

[mlir][EmitC]Add a Reflection Map to a Class #150572

Open

ilovepi reviewed Aug 6, 2025

View reviewed changes

Jaddyen requested review from aniragil and ilovepi August 7, 2025 04:27

ilovepi reviewed Aug 7, 2025

View reviewed changes

aniragil requested changes Aug 7, 2025

View reviewed changes

Jaddyen added 8 commits August 7, 2025 23:20

Adding lowering for copyop

5a54bdf

use subscript and apply ops

b5b7637

allow for multi dimensional arays

c17cf73

update test file

a7a2d01

test cpp output

12e967e

ensure both headers are added

5870e6b

update test file

3663062

ensure no duplicate headers

ec252fc

Jaddyen force-pushed the memref-ops-copyop branch from 726059d to ec252fc Compare August 8, 2025 00:16

Jaddyen requested review from aniragil and ilovepi August 8, 2025 12:58

ilovepi approved these changes Aug 8, 2025

View reviewed changes

refactoring

de3c63d

Jaddyen commented Aug 8, 2025

View reviewed changes

aniragil requested changes Aug 9, 2025

View reviewed changes

nit clean up

ee8ab84

Jaddyen requested a review from aniragil August 13, 2025 17:33

aniragil approved these changes Aug 14, 2025

View reviewed changes

Jaddyen merged commit bfda0e7 into llvm:main Aug 14, 2025
9 checks passed

rupprecht pushed a commit that referenced this pull request Aug 14, 2025

[bazel][MLIR] Fix for MemRefToEmitC for #151206 (#153590)

7d91213

Jaddyen deleted the memref-ops-copyop branch September 2, 2025 20:42

	// NOCPP-NEXT: emitc.include <"string.h">
	// NOCPP: emitc.include <"string.h">

	// CPP: module {
	// CPP-NEXT: emitc.include <"cstring">
	// CPP: emitc.include <"cstring">

-// NOCPP: module {
-// NOCPP-NEXT:  emitc.include <"string.h">
-// NOCPP-NEXT:  emitc.include <"stdlib.h">
-// CPP: module {
-// CPP-NEXT:  emitc.include <"cstring">
-// CPP-NEXT:  emitc.include <"cstdlib">
+// CHECK: module {
+// NOCPP:  emitc.include <"string.h">
+// NOCPP-NEXT:  emitc.include <"stdlib.h">
+// CPP:  emitc.include <"cstring">
+// CPP-NEXT:  emitc.include <"cstdlib">

	Value calculateMemrefTotalSizeBytes(Location loc, MemRefType memrefType,
	static Value calculateMemrefTotalSizeBytes(Location loc, MemRefType memrefType,

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering CopyOp #151206

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering CopyOp #151206

Uh oh!

Conversation

Jaddyen commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `CopyOp` #151206

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `CopyOp` #151206

Jaddyen commented Jul 29, 2025 •

edited

Loading

llvmbot commented Jul 29, 2025 •

edited

Loading