[mlir][EmitC]Expand the MemRefToEmitC pass - Adding scalars by Jaddyen · Pull Request #148055 · llvm/llvm-project

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

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

memref.global "private" constant @__constant_xi32 : memref<i32> = dense<-1>
func.func @globals() {
    memref.get_global @__constant_xi32 : memref<i32>
}

To:

emitc.global static const @__constant_xi32 : i32 = -1
    emitc.func @globals() {
      %0 = get_global @__constant_xi32 : !emitc.lvalue<i32>
      %1 = apply "&"(%0) : (!emitc.lvalue<i32>) -> !emitc.ptr<i32>
      return
    }

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

Suggested change

MemRefType type = op.getType();

MemRefType opTy = op.getType();

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

I agree. Thanks for the pointer!

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

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

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

Def! Thanks for the pointer.

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

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

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 https://github.com/llvm/llvm-project/blob/36b61a6b5731a524b4e79e77d7505c7a5ef3d0f9/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir#L46 going to work now?

Not yet. This would require changing those particular ops. I could circle back to this once we can completely lower the inliner model.

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

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

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

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

What is the initialValue before this vs splat value returned?

Without using getSplatValue, we have the initial value as: initial_value = dense<-1> : tensor<i32>
After getting the splat value, we have emitc.global static const @__constant_xi32 : i32 = -1

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

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

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

memref.getGlobal gets converted to emitc.getglobal but emitc.getglobal only returns LValue or Array. So in the case that we have a constant, we create an LValue, else we return an array.

Note that in general lowering getglobal to lvalue doesn't work correctly when the result is passed to function calls for example. So I would expect rank 0 memrefs to be lowered to pointers (at least when it might escape).

Thanks for pointing this out. I've updated the conversion to reflect this.

Sorry für being unclear. I think globals should still be lowered to lvalues so that it allocates the necessary storage. But the get_global may be lowered to EmitC.get_global + EmitC.apply "&" to get a Pointer to the variable. But I haven't tried this out.

What I've tried and been able to implement is the conversion:
From:

memref.global "private" constant @__constant_xi32 : memref<i32> = dense<-1> func.func @globals() { memref.get_global @__constant_xi32 : memref<i32> }

To:

emitc.global static const @__constant_xi32 : i32 = -1 emitc.func @globals() { %0 = get_global @__constant_xi32 : !emitc.lvalue<i32> %1 = apply "&"(%0) : (!emitc.lvalue<i32>) -> !emitc.ptr<i32> return }

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

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

Don't you have this check just below too?

I do! Thanks for the pointer.

jpienaar

Looks good in general. I believe you also tested it with compiling the output too.

simon-camp

LGTM

Jaddyen · 2025-07-18T04:30:27Z

Looks good in general. I believe you also tested it with compiling the output too.

Yes, I did!

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

jpienaar approved these changes Jul 16, 2025

View reviewed changes

Jaddyen requested a review from simon-camp July 17, 2025 19:30

Jaddyen added 8 commits July 17, 2025 19:44

Initial work on memref ops

a86bd7a

Convert scalars to constants

d62dd8d

global and getGlobal

263d6e0

Use a helper when you can

5612d99

Lower to pointer type

2950fd0

Use an lvalue and apply op

82bd2b4

Update test file

1dc0e63

remve unncessary auto

bc7b8e2

ajaden-codes force-pushed the memref-ops branch from b77ba65 to bc7b8e2 Compare July 17, 2025 19:56

simon-camp approved these changes Jul 17, 2025

View reviewed changes

Merge branch 'main' into memref-ops

40e80d5

Jaddyen changed the title ~~Expand the MemRefToEmitC pass - Adding scalars~~ [mlir][emitc]Expand the MemRefToEmitC pass - Adding scalars Jul 18, 2025

Jaddyen changed the title ~~[mlir][emitc]Expand the MemRefToEmitC pass - Adding scalars~~ [mlir][EmitC]Expand the MemRefToEmitC pass - Adding scalars Jul 18, 2025

Jaddyen merged commit 7fd91bb into llvm:main Jul 18, 2025
9 checks passed

Jaddyen deleted the memref-ops branch July 18, 2025 17:25

This was referenced Jul 23, 2025

test abhinavgaba/llvm-project#2

Closed

Add dataFence plugin interface abhinavgaba/llvm-project#3

Closed

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

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!

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!

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

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!

jpienaar left a comment

Choose a reason for hiding this comment

Uh oh!

simon-camp left a comment

Choose a reason for hiding this comment

Uh oh!

Jaddyen commented Jul 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Jaddyen commented Jul 10, 2025 •

edited

Loading

llvmbot commented Jul 10, 2025 •

edited

Loading

Jaddyen Jul 14, 2025 •

edited

Loading