Skip to content

Conversation

@Vladislave0-0
Copy link
Contributor

This PR marks emitc.literal as a valid operation within emitc.expression, removing an artificial restriction since literals are inherently inlined into expressions during emission.

Example:

%0 = emitc.expression : i32 {
  %literal = emitc.literal "42" : i32
  emitc.yield %literal : i32
}

@llvmbot
Copy link
Member

llvmbot commented Aug 27, 2025

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Vlad Lazar (Vladislave0-0)

Changes

This PR marks emitc.literal as a valid operation within emitc.expression, removing an artificial restriction since literals are inherently inlined into expressions during emission.

Example:

%0 = emitc.expression : i32 {
  %literal = emitc.literal "42" : i32
  emitc.yield %literal : i32
}

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

4 Files Affected:

  • (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.td (+1-1)
  • (modified) mlir/lib/Target/Cpp/TranslateToCpp.cpp (+15-8)
  • (modified) mlir/test/Dialect/EmitC/form-expressions.mlir (+26)
  • (modified) mlir/test/Target/Cpp/expressions.mlir (+26)
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 59beac7d64154..6fbf52b16db31 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -851,7 +851,7 @@ def EmitC_IncludeOp
   let hasCustomAssemblyFormat = 1;
 }
 
-def EmitC_LiteralOp : EmitC_Op<"literal", [Pure]> {
+def EmitC_LiteralOp : EmitC_Op<"literal", [Pure, CExpressionInterface]> {
   let summary = "Literal operation";
   let description = [{
     The `emitc.literal` operation produces an SSA value equal to some constant
diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index 7284e24776175..86fa7b0eac4c0 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -98,6 +98,7 @@ static FailureOr<int> getOperatorPrecedence(Operation *operation) {
       .Case<emitc::ConditionalOp>([&](auto op) { return 2; })
       .Case<emitc::DivOp>([&](auto op) { return 13; })
       .Case<emitc::LoadOp>([&](auto op) { return 16; })
+      .Case<emitc::LiteralOp>([&](auto op) { return 16; })
       .Case<emitc::LogicalAndOp>([&](auto op) { return 4; })
       .Case<emitc::LogicalNotOp>([&](auto op) { return 15; })
       .Case<emitc::LogicalOrOp>([&](auto op) { return 3; })
@@ -440,6 +441,16 @@ static LogicalResult printOperation(CppEmitter &emitter, emitc::LoadOp loadOp) {
   return emitter.emitOperand(loadOp.getOperand());
 }
 
+static LogicalResult printOperation(CppEmitter &emitter,
+                                    emitc::LiteralOp literalOp) {
+  // If literalOp is used inside an expression, we treat it as an embedded one.
+  if (emitter.isPartOfCurrentExpression(literalOp.getResult()))
+    return emitter.ostream() << literalOp.getValue(), success();
+
+  emitter.cacheDeferredOpResult(literalOp.getResult(), literalOp.getValue());
+  return success();
+}
+
 static LogicalResult printBinaryOperation(CppEmitter &emitter,
                                           Operation *operation,
                                           StringRef binaryOperator) {
@@ -1688,10 +1699,10 @@ LogicalResult CppEmitter::emitOperation(Operation &op, bool trailingSemicolon) {
                 emitc::DeclareFuncOp, emitc::DivOp, emitc::ExpressionOp,
                 emitc::FieldOp, emitc::FileOp, emitc::ForOp, emitc::FuncOp,
                 emitc::GlobalOp, emitc::IfOp, emitc::IncludeOp, emitc::LoadOp,
-                emitc::LogicalAndOp, emitc::LogicalNotOp, emitc::LogicalOrOp,
-                emitc::MulOp, emitc::RemOp, emitc::ReturnOp, emitc::SubOp,
-                emitc::SwitchOp, emitc::UnaryMinusOp, emitc::UnaryPlusOp,
-                emitc::VariableOp, emitc::VerbatimOp>(
+                emitc::LiteralOp, emitc::LogicalAndOp, emitc::LogicalNotOp,
+                emitc::LogicalOrOp, emitc::MulOp, emitc::RemOp, emitc::ReturnOp,
+                emitc::SubOp, emitc::SwitchOp, emitc::UnaryMinusOp,
+                emitc::UnaryPlusOp, emitc::VariableOp, emitc::VerbatimOp>(
 
               [&](auto op) { return printOperation(*this, op); })
           // Func ops.
@@ -1705,10 +1716,6 @@ LogicalResult CppEmitter::emitOperation(Operation &op, bool trailingSemicolon) {
             cacheDeferredOpResult(op.getResult(), op.getFieldName());
             return success();
           })
-          .Case<emitc::LiteralOp>([&](auto op) {
-            cacheDeferredOpResult(op.getResult(), op.getValue());
-            return success();
-          })
           .Case<emitc::MemberOp>([&](auto op) {
             cacheDeferredOpResult(op.getResult(), createMemberAccess(op));
             return success();
diff --git a/mlir/test/Dialect/EmitC/form-expressions.mlir b/mlir/test/Dialect/EmitC/form-expressions.mlir
index 1ac053bf1215f..e4e8fc0ebbc00 100644
--- a/mlir/test/Dialect/EmitC/form-expressions.mlir
+++ b/mlir/test/Dialect/EmitC/form-expressions.mlir
@@ -160,3 +160,29 @@ func.func @expression_with_load(%arg0: i32, %arg1: !emitc.ptr<i32>) -> i1 {
   %c = emitc.cmp lt, %b, %arg0 :(i32, i32) -> i1
   return %c : i1
 }
+
+// CHECK-LABEL:   func.func @expression_with_literal(
+// CHECK-SAME:      %[[ARG0:.*]]: i32) -> i1 {
+// CHECK:           %[[VAL_0:.*]] = emitc.expression : i32 {
+// CHECK:             %[[VAL_1:.*]] = literal "1" : i32
+// CHECK:             yield %[[VAL_1]] : i32
+// CHECK:           }
+// CHECK:           %[[VAL_2:.*]] = emitc.expression : i32 {
+// CHECK:             %[[VAL_3:.*]] = literal "2" : i32
+// CHECK:             yield %[[VAL_3]] : i32
+// CHECK:           }
+// CHECK:           %[[VAL_4:.*]] = emitc.expression : i1 {
+// CHECK:             %[[VAL_5:.*]] = add %[[VAL_0]], %[[VAL_2]] : (i32, i32) -> i32
+// CHECK:             %[[VAL_6:.*]] = cmp lt, %[[VAL_5]], %[[ARG0]] : (i32, i32) -> i1
+// CHECK:             yield %[[VAL_6]] : i1
+// CHECK:           }
+// CHECK:           return %[[VAL_4]] : i1
+// CHECK:         }
+
+func.func @expression_with_literal(%arg0: i32) -> i1 {
+  %literal1 = emitc.literal "1" : i32
+  %literal2 = emitc.literal "2" : i32
+  %b = emitc.add %literal1, %literal2 : (i32, i32) -> i32
+  %c = emitc.cmp lt, %b, %arg0 :(i32, i32) -> i1
+  return %c : i1
+}
diff --git a/mlir/test/Target/Cpp/expressions.mlir b/mlir/test/Target/Cpp/expressions.mlir
index 5057d6ca0c4cf..b5cea8e5e9bbf 100644
--- a/mlir/test/Target/Cpp/expressions.mlir
+++ b/mlir/test/Target/Cpp/expressions.mlir
@@ -439,3 +439,29 @@ emitc.func @expression_with_call_opaque_with_args_array(%0 : i32, %1 : i32) {
   }
   return
 }
+
+
+// CPP-DEFAULT:      bool expression_with_literal(int32_t [[VAL_1:v[0-9]+]]) {
+// CPP-DEFAULT-NEXT:   bool [[VAL_2:v[0-9]+]] = (1 + [[VAL_1]]) / 2 < 3;
+// CPP-DEFAULT-NEXT:   return [[VAL_2]];
+// CPP-DEFAULT-NEXT: }
+
+// CPP-DECLTOP:      bool expression_with_literal(int32_t [[VAL_1:v[0-9]+]]) {
+// CPP-DECLTOP-NEXT:   bool [[VAL_2:v[0-9]+]];
+// CPP-DECLTOP-NEXT:   [[VAL_2]] = (1 + [[VAL_1]]) / 2 < 3;
+// CPP-DECLTOP-NEXT:   return [[VAL_2]];
+// CPP-DECLTOP-NEXT: }
+
+func.func @expression_with_literal(%arg0 : i32) -> i1 {
+  %ret = emitc.expression noinline : i1 {
+    %literal1 = emitc.literal "1" : i32
+    %literal2 = emitc.literal "2" : i32
+    %add = add %literal1, %arg0 : (i32, i32) -> i32
+    %div = div %add, %literal2 : (i32, i32) -> i32
+    %literal3 = emitc.literal "3" : i32
+    %f = emitc.cmp lt, %div, %literal3 :(i32, i32) -> i1
+    emitc.yield %f : i1
+  }
+
+  return %ret : i1
+}

@aniragil
Copy link
Contributor

Literal is marked pure. Won't this expose the expression to CSE, causing the example above to be left without any operation with the expression? (see this related discussion)

@Vladislave0-0
Copy link
Contributor Author

Literal is marked pure. Won't this expose the expression to CSE, causing the example above to be left without any operation with the expression? (see this related discussion)

Thanks! If I understood your question correctly, I think there should be no problem with CSE, because emitc.expression creates an isolated region that protects internal operations from CSE with external operations. So the test above will work correctly.

@aniragil
Copy link
Contributor

Thanks! If I understood your question correctly, I think there should be no problem with CSE, because emitc.expression creates an isolated region that protects internal operations from CSE with external operations. So the test above will work correctly.

Actually emitc.expression doesn't have the isolated-from-above trait. I've just uploaded this PR that does that following the discussion in that PR, would be great if you could also take a look.

@kchibisov
Copy link
Contributor

kchibisov commented Aug 30, 2025

What do you think about ignoring literal in form-expression pass? I'd say that the point of this PR is that if your literal somehow ended in emitc.expression due to other transformations, to not blow up, but generally, we don't need it in the expression itself, because it's inlined anyway, so we just make things a bit more messy.

Just checked a bit more, and it indeed does change the way code is generated, because inline from the CExpressionInterface doesn't match the way literal does so, so I'd argue to not wrap literals, to preserve old behavior with literals.

@Vladislave0-0 Vladislave0-0 force-pushed the add-literal-to-expression branch from bbe2aa9 to 6a27d24 Compare September 2, 2025 22:13
This PR marks `emitc.literal` as a valid operation within
`emitc.expression`, removing an artificial restriction since
literals are inherently inlined into expressions during emission.

Example:
```mlir
%0 = emitc.expression : i32 {
  %literal = emitc.literal "42" : i32
  emitc.yield %literal : i32
}
```
Literal is always inlined, so there's no need to form expressions
with it as well.
@Vladislave0-0 Vladislave0-0 force-pushed the add-literal-to-expression branch from 7cffbec to e220359 Compare September 7, 2025 14:39
@Vladislave0-0
Copy link
Contributor Author

@aniragil do I need to close this PR?

@aniragil
Copy link
Contributor

@aniragil do I need to close this PR?

Let's first see if my PR is accepted (would be great if you could also take a look at it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants