Expand the MemRefToEmitC pass - Adding scalars #148055

Jaddyen · 2025-07-10T20:39:03Z

This aims to expand the the MemRefToEmitC pass so that it can accept global scalars.

llvmbot · 2025-07-10T23:51:21Z

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Jaden Angella (Jaddyen)

Changes

This aims to expand the the MemRefToEmitC pass so that it can accept global scalars.

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

2 Files Affected:

(modified) mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp (+25-3)
(modified) mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir (+4)

diff --git a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
index db244d1d1cac8..e55c8e48ad105 100644
--- a/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
+++ b/mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp
@@ -16,7 +16,9 @@
 #include "mlir/Dialect/EmitC/IR/EmitC.h"
 #include "mlir/Dialect/MemRef/IR/MemRef.h"
 #include "mlir/IR/Builders.h"
+#include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/PatternMatch.h"
+#include "mlir/IR/TypeRange.h"
 #include "mlir/Transforms/DialectConversion.h"
 
 using namespace mlir;
@@ -83,7 +85,7 @@ struct ConvertGlobal final : public OpConversionPattern<memref::GlobalOp> {
   LogicalResult
   matchAndRewrite(memref::GlobalOp op, OpAdaptor operands,
                   ConversionPatternRewriter &rewriter) const override {
-
+    MemRefType type = op.getType();
     if (!op.getType().hasStaticShape()) {
       return rewriter.notifyMatchFailure(
           op.getLoc(), "cannot transform global with dynamic shape");
@@ -95,7 +97,13 @@ struct ConvertGlobal final : public OpConversionPattern<memref::GlobalOp> {
           op.getLoc(), "global variable with alignment requirement is "
                        "currently not supported");
     }
-    auto resultTy = getTypeConverter()->convertType(op.getType());
+
+    Type resultTy;
+    if (type.getRank() == 0)
+      resultTy = getTypeConverter()->convertType(type.getElementType());
+    else
+      resultTy = getTypeConverter()->convertType(type);
+
     if (!resultTy) {
       return rewriter.notifyMatchFailure(op.getLoc(),
                                          "cannot convert result type");
@@ -114,6 +122,10 @@ struct ConvertGlobal final : public OpConversionPattern<memref::GlobalOp> {
     bool externSpecifier = !staticSpecifier;
 
     Attribute initialValue = operands.getInitialValueAttr();
+    if (type.getRank() == 0) {
+      auto elementsAttr = llvm::cast<ElementsAttr>(*op.getInitialValue());
+      initialValue = elementsAttr.getSplatValue<Attribute>();
+    }
     if (isa_and_present<UnitAttr>(initialValue))
       initialValue = {};
 
@@ -132,7 +144,17 @@ struct ConvertGetGlobal final
   matchAndRewrite(memref::GetGlobalOp op, OpAdaptor operands,
                   ConversionPatternRewriter &rewriter) const override {
 
-    auto resultTy = getTypeConverter()->convertType(op.getType());
+    MemRefType type = op.getType();
+    Type resultTy;
+    if (type.getRank() == 0)
+      resultTy = emitc::LValueType::get(
+          getTypeConverter()->convertType(type.getElementType()));
+    else
+      resultTy = getTypeConverter()->convertType(type);
+
+    if (!resultTy)
+      return rewriter.notifyMatchFailure(op.getLoc(), "cannot convert type");
+
     if (!resultTy) {
       return rewriter.notifyMatchFailure(op.getLoc(),
                                          "cannot convert result type");
diff --git a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir
index d37fd1de90add..445a28534325a 100644
--- a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir
+++ b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc.mlir
@@ -41,6 +41,8 @@ func.func @memref_load(%buff : memref<4x8xf32>, %i: index, %j: index) -> f32 {
 module @globals {
   memref.global "private" constant @internal_global : memref<3x7xf32> = dense<4.0>
   // CHECK-NEXT: emitc.global static const @internal_global : !emitc.array<3x7xf32> = dense<4.000000e+00>
+  memref.global "private" constant @__constant_xi32 : memref<i32> = dense<-1>
+  // CHECK-NEXT: emitc.global static const @__constant_xi32 : i32 = -1
   memref.global @public_global : memref<3x7xf32>
   // CHECK-NEXT: emitc.global extern @public_global : !emitc.array<3x7xf32>
   memref.global @uninitialized_global : memref<3x7xf32> = uninitialized
@@ -50,6 +52,8 @@ module @globals {
   func.func @use_global() {
     // CHECK-NEXT: emitc.get_global @public_global : !emitc.array<3x7xf32>
     %0 = memref.get_global @public_global : memref<3x7xf32>
+    // CHECK- NEXT: emitc.get_global @__constant_xi32 : !emitc.lvalue<i32>
+    %1 = memref.get_global @__constant_xi32 : memref<i32>
     return
   }
 }

ilovepi · 2025-07-11T17:39:56Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

@@ -83,7 +85,7 @@ struct ConvertGlobal final : public OpConversionPattern<memref::GlobalOp> {
  LogicalResult
  matchAndRewrite(memref::GlobalOp op, OpAdaptor operands,
                  ConversionPatternRewriter &rewriter) const override {
-
+    MemRefType type = op.getType();


Suggested change

MemRefType type = op.getType();

MemRefType opTy = op.getType();

I'm not sure we want to use type as a variable name...

ilovepi · 2025-07-11T17:41:14Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+    if (type.getRank() == 0)
+      resultTy = getTypeConverter()->convertType(type.getElementType());
+    else
+      resultTy = getTypeConverter()->convertType(type);


Maybe introduce a helper, since I see a similar pattern in a few spots?

ilovepi · 2025-07-11T17:46:12Z

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

@@ -41,6 +41,8 @@ func.func @memref_load(%buff : memref<4x8xf32>, %i: index, %j: index) -> f32 {
 module @globals {
  memref.global "private" constant @internal_global : memref<3x7xf32> = dense<4.0>
  // CHECK-NEXT: emitc.global static const @internal_global : !emitc.array<3x7xf32> = dense<4.000000e+00>
+  memref.global "private" constant @__constant_xi32 : memref<i32> = dense<-1>


I was expecting to see a corresponding change to memref-to-emitc-failed.mlir. Looking I suppose it isn't there, but are any of the cases, like

llvm-project/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir

Line 46 in 36b61a6

%0 = memref.alloca() : memref<0xf32>

going to work now?

jpienaar · 2025-07-11T20:16:55Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+
+    Type resultTy;
+    if (type.getRank() == 0)
+      resultTy = getTypeConverter()->convertType(type.getElementType());


You should be able to just do

resultTy = getTypeConverter()->convertType(getElementTypeOrSelf(type));

jpienaar · 2025-07-11T20:18:57Z

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

@@ -50,6 +52,8 @@ module @globals {
  func.func @use_global() {
    // CHECK-NEXT: emitc.get_global @public_global : !emitc.array<3x7xf32>
    %0 = memref.get_global @public_global : memref<3x7xf32>
+    // CHECK- NEXT: emitc.get_global @__constant_xi32 : !emitc.lvalue<i32>
+    %1 = memref.get_global @__constant_xi32 : memref<i32>


Just to check, so previously memref<1xi32> worked, but memref<i32> didn't work before this?

jpienaar · 2025-07-11T20:19:46Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

@@ -114,6 +122,10 @@ struct ConvertGlobal final : public OpConversionPattern<memref::GlobalOp> {
    bool externSpecifier = !staticSpecifier;

    Attribute initialValue = operands.getInitialValueAttr();
+    if (type.getRank() == 0) {
+      auto elementsAttr = llvm::cast<ElementsAttr>(*op.getInitialValue());
+      initialValue = elementsAttr.getSplatValue<Attribute>();


What is the initialValue before this vs splat value returned?

jpienaar · 2025-07-11T20:20:04Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+    MemRefType type = op.getType();
+    Type resultTy;
+    if (type.getRank() == 0)
+      resultTy = emitc::LValueType::get(


Why does this need to be LValue, while one below not?

jpienaar · 2025-07-11T20:20:27Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+      resultTy = getTypeConverter()->convertType(type);
+
+    if (!resultTy)
+      return rewriter.notifyMatchFailure(op.getLoc(), "cannot convert type");


Don't you have this check just below too?

Jaddyen added 3 commits July 7, 2025 18:49

Initial work on memref ops

127a1ed

Convert scalars to constants

c237251

global and getGlobal

36b61a6

Jaddyen changed the title ~~Expand the MemRef to EmitC pass~~ Expand the MemRef to EmitC pass - Adding scalars Jul 10, 2025

Jaddyen changed the title ~~Expand the MemRef to EmitC pass - Adding scalars~~ Expand the MemRefToEmitC pass - Adding scalars Jul 10, 2025

Jaddyen marked this pull request as ready for review July 10, 2025 23:50

llvmbot added mlir mlir:emitc labels Jul 10, 2025

ilovepi reviewed Jul 11, 2025

View reviewed changes

jpienaar reviewed Jul 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Expand the MemRefToEmitC pass - Adding scalars #148055

Expand the MemRefToEmitC pass - Adding scalars #148055

Jaddyen commented Jul 10, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 10, 2025 •

edited

Loading

Uh oh!

ilovepi Jul 11, 2025

Uh oh!

ilovepi Jul 11, 2025

Uh oh!

ilovepi Jul 11, 2025

Uh oh!

jpienaar Jul 11, 2025

Uh oh!

jpienaar Jul 11, 2025

Uh oh!

jpienaar Jul 11, 2025

Uh oh!

jpienaar Jul 11, 2025

Uh oh!

jpienaar Jul 11, 2025

Uh oh!

Uh oh!

	MemRefType type = op.getType();
	MemRefType opTy = op.getType();

Expand the MemRefToEmitC pass - Adding scalars #148055

Are you sure you want to change the base?

Expand the MemRefToEmitC pass - Adding scalars #148055

Conversation

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

Uh oh!

Uh oh!

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

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!

Uh oh!

Jaddyen commented Jul 10, 2025 •

edited

Loading

llvmbot commented Jul 10, 2025 •

edited

Loading