[Mlir-commits] [mlir] [mlir][emitc] mark `emitc.load` with `CExpression` (PR #130802)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Kirill Chibisov (kchibisov)

<details>
<summary>Changes</summary>

Follow the `call` and `call_opaque` operations, as well as `apply`, which already are marked as `CExpression` even though they have side effects.

Even though `emitc.load` can be included inside the `emitc.expression`, the inlining and `--form-expression` pass won't actually inline them inside other expression due to it having a side effect, thus unless the user manually writes the `emitc.load` inside the `emitc.expression` it won't appear there.

--

It was brought https://github.com/llvm/llvm-project/pull/91475#issuecomment-2302529428 and while there was some opposition due to `load` having a side effect, `emitc` already allows all the rest operations that have it, so for consistency reasons, enabling it doesn't really hurt from my point of view. Especially given that `--form-expression` doesn't allow
it to really inline inside other expressions, which makes sense, since if the users want such behavior, they should explicitly opt-in.

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


3 Files Affected:

- (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.td (+1-1) 
- (modified) mlir/lib/Target/Cpp/TranslateToCpp.cpp (+1) 
- (modified) mlir/test/Dialect/EmitC/transforms.mlir (+23) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index a518e7425956c..af2eed81e0d5b 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -927,7 +927,7 @@ def EmitC_LogicalOrOp : EmitC_BinaryOp<"logical_or", [CExpression]> {
   let assemblyFormat = "operands attr-dict `:` type(operands)";
 }
 
-def EmitC_LoadOp : EmitC_Op<"load", [
+def EmitC_LoadOp : EmitC_Op<"load", [CExpression,
   TypesMatchWith<"result type matches value type of 'operand'",
                   "operand", "result",
                   "::llvm::cast<LValueType>($_self).getValueType()">
diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index b00820ffc542b..a3761ead4adde 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -100,6 +100,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::LogicalAndOp>([&](auto op) { return 4; })
       .Case<emitc::LogicalNotOp>([&](auto op) { return 15; })
       .Case<emitc::LogicalOrOp>([&](auto op) { return 3; })
diff --git a/mlir/test/Dialect/EmitC/transforms.mlir b/mlir/test/Dialect/EmitC/transforms.mlir
index d204dec70d449..0678b4e5515c0 100644
--- a/mlir/test/Dialect/EmitC/transforms.mlir
+++ b/mlir/test/Dialect/EmitC/transforms.mlir
@@ -129,3 +129,26 @@ func.func @single_result_requirement() -> (i32, i32) {
   %0:2 = emitc.call_opaque "foo" () : () -> (i32, i32)
   return %0#0, %0#1 : i32, i32
 }
+
+// CHECK-LABEL: func.func @expression_with_load(
+// CHECK-SAME:      %[[VAL_0:.*]]: i32, %[[VAL_1:.*]]: i32) -> i1 {
+// CHECK:         %[[VAL_2:.*]] = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
+// CHECK:         %[[VAL_3:.*]] = emitc.expression : i32 {
+// CHECK:           %[[VAL_4:.*]] = load %[[VAL_2]] : <i32>
+// CHECK:           yield %[[VAL_4]] : i32
+// CHECK:         }
+// CHECK:         %[[VAL_5:.*]] = emitc.expression : i1 {
+// CHECK:           %[[VAL_6:.*]] = add %[[VAL_3]], %[[VAL_1]] : (i32, i32) -> i32
+// CHECK:           %[[VAL_7:.*]] = cmp lt, %[[VAL_6]], %[[VAL_0]] : (i32, i32) -> i1
+// CHECK:           yield %[[VAL_7]] : i1
+// CHECK:         }
+// CHECK:         return %[[VAL_5]] : i1
+// CHECK:       }
+
+func.func @expression_with_load(%arg0: i32, %arg1: i32) -> i1 {
+  %0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
+  %a = emitc.load %0 : !emitc.lvalue<i32>
+  %b = emitc.add %a, %arg1 : (i32, i32) -> i32
+  %c = emitc.cmp lt, %b, %arg0 :(i32, i32) -> i1
+  return %c : i1
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/130802