[MLIR][EmitC] Don't translate expressions inline if user is emitc.subscript

[christopherbate]

This change updates the logic that determines whether an emitc.expression
result is translated into a dedicated variable assignment. Due to how the
translation of emitc.subscript currently works, a previously inline-able
emitc.expression would produce incorrect C++ if its single user was a
emitc.subscript operation.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-emitc

Author: Chris (christopherbate)

Changes

This change updates the logic that determines whether an emitc.expression
result is translated into a dedicated variable assignment. Due to how the
translation of emitc.subscript currently works, a previously inline-able
emitc.expression would produce incorrect C++ if its single user was a
emitc.subscript operation.

---

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

2 Files Affected:

• (modified) mlir/lib/Target/Cpp/TranslateToCpp.cpp (+8-1)
• (modified) mlir/test/Target/Cpp/expressions.mlir (+15)

diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index 1bdb74cd8bf2e4..7db7163bac4ab6 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -293,9 +293,16 @@ static bool shouldBeInlined(ExpressionOp expressionOp) {
   if (!result.hasOneUse())
     return false;
 
+  Operation *user = *result.getUsers().begin();
+
+  // Do not inline expressions used by subscript operations, since the
+  // way the subscript operation translation is implemented requires that
+  // variables be materialized.
+  if (isa<emitc::SubscriptOp>(user))
+    return false;
+
   // Do not inline expressions used by other expressions, as any desired
   // expression folding was taken care of by transformations.
-  Operation *user = *result.getUsers().begin();
   return !user->getParentOfType<ExpressionOp>();
 }
 
diff --git a/mlir/test/Target/Cpp/expressions.mlir b/mlir/test/Target/Cpp/expressions.mlir
index 9ec9dcc3c6a84b..2fc84ae2b7d6fb 100644
--- a/mlir/test/Target/Cpp/expressions.mlir
+++ b/mlir/test/Target/Cpp/expressions.mlir
@@ -210,3 +210,18 @@ func.func @expression_with_address_taken(%arg0: i32, %arg1: i32, %arg2: !emitc.p
   }
   return %c : i1
 }
+
+// CPP-DEFAULT: int32_t expression_with_subscript_user(void* [[VAL_1:v.+]])
+// CPP-DEFAULT-NEXT:   int64_t [[VAL_2:v.+]] = 0;
+// CPP-DEFAULT-NEXT:   int32_t* [[VAL_3:v.+]] = (int32_t*) [[VAL_1]];
+// CPP-DEFAULT-NEXT:   return [[VAL_3]]{{\[}}[[VAL_2]]{{\]}};
+
+func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void">>) -> i32 {
+  %c0 = "emitc.constant"() {value = 0 : i64} : () -> i64
+  %0 = emitc.expression : !emitc.ptr<i32> {
+    %0 = emitc.cast %arg0 : !emitc.ptr<!emitc.opaque<"void">> to !emitc.ptr<i32>
+    emitc.yield %0 : !emitc.ptr<i32>
+  }
+  %1 = emitc.subscript %0[%c0] : (!emitc.ptr<i32>, i64) -> i32
+  return %1 : i32
+}
\ No newline at end of file

[mgehre-amd]

Good catch!
I think the lvalue representation will fix this issue properly.