[mlir][emitc] Mark emitc.literal with CExpression

[Vladislave0-0]

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]

@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]

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]

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]

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]

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]

@aniragil do I need to close this PR?

[aniragil]

@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)