[CIR] Upstream l-value emission for ExprWithCleanups

[andykaylor]

This adds the necessary handler for emitting an l-value for an ExprWithCleanups expression.

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This adds the necessary handler for emitting an l-value for an ExprWithCleanups expression.

---

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

2 Files Affected:

• (modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+29)
• (added) clang/test/CIR/CodeGen/temporary-materialization.cpp (+98)

diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 5d5209b9ffb60..4a4e9e5d054ed 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -912,6 +912,35 @@ LValue CIRGenFunction::emitLValue(const Expr *e) {
   case Expr::CXXOperatorCallExprClass:
   case Expr::UserDefinedLiteralClass:
     return emitCallExprLValue(cast<CallExpr>(e));
+  case Expr::ExprWithCleanupsClass: {
+    const auto *cleanups = cast<ExprWithCleanups>(e);
+    LValue lv;
+
+    mlir::Location scopeLoc = getLoc(e->getSourceRange());
+    [[maybe_unused]] auto scope = cir::ScopeOp::create(
+        builder, scopeLoc, /*scopeBuilder=*/
+        [&](mlir::OpBuilder &b, mlir::Location loc) {
+          CIRGenFunction::LexicalScope lexScope{*this, loc,
+                                                builder.getInsertionBlock()};
+
+          lv = emitLValue(cleanups->getSubExpr());
+          if (lv.isSimple()) {
+            // Defend against branches out of gnu statement expressions
+            // surrounded by cleanups.
+            Address addr = lv.getAddress();
+            mlir::Value v = addr.getPointer();
+            assert(!cir::MissingFeatures::addressIsKnownNonNull());
+            assert(!cir::MissingFeatures::opTBAA());
+            assert(!cir::MissingFeatures::objCGC());
+            lv = LValue::makeAddr(addr.withPointer(v), lv.getType(),
+                                  lv.getBaseInfo());
+          }
+        });
+
+    // FIXME: Is it possible to create an ExprWithCleanups that produces a
+    // bitfield lvalue or some other non-simple lvalue?
+    return lv;
+  }
   case Expr::ParenExprClass:
     return emitLValue(cast<ParenExpr>(e)->getSubExpr());
   case Expr::GenericSelectionExprClass:
diff --git a/clang/test/CIR/CodeGen/temporary-materialization.cpp b/clang/test/CIR/CodeGen/temporary-materialization.cpp
new file mode 100644
index 0000000000000..e7088f8bb4d9a
--- /dev/null
+++ b/clang/test/CIR/CodeGen/temporary-materialization.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefix=LLVM
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+int make_int();
+
+int test() {
+  const int &x = make_int();
+  return x;
+}
+
+//      CIR: cir.func {{.*}} @_Z4testv()
+//      CIR:   %[[TEMP_SLOT:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["ref.tmp0", init]
+// CIR-NEXT:   %[[X:.*]] = cir.alloca !cir.ptr<!s32i>, !cir.ptr<!cir.ptr<!s32i>>, ["x", init, const]
+// CIR-NEXT:   cir.scope {
+// CIR-NEXT:     %[[TEMP_VALUE:.*]] = cir.call @_Z8make_intv() : () -> !s32i
+// CIR-NEXT:     cir.store{{.*}} %[[TEMP_VALUE]], %[[TEMP_SLOT]]
+// CIR-NEXT:   }
+// CIR-NEXT:   cir.store{{.*}} %[[TEMP_SLOT]], %[[X]]
+
+// LLVM: define {{.*}} i32 @_Z4testv()
+// LLVM:   %[[RETVAL:.*]] = alloca i32
+// LLVM:   %[[TEMP_SLOT:.*]] = alloca i32
+// LLVM:   %[[X:.*]] = alloca ptr
+// LLVM:   br label %[[SCOPE_LABEL:.*]]
+// LLVM: [[SCOPE_LABEL]]:
+// LLVM:   %[[TEMP_VALUE:.*]] = call i32 @_Z8make_intv()
+// LLVM:   store i32 %[[TEMP_VALUE]], ptr %[[TEMP_SLOT]]
+// LLVM:   br label %[[SCOPE_END_LABEL:.*]]
+// LLVM: [[SCOPE_END_LABEL]]:
+// LLVM:   store ptr %[[TEMP_SLOT]], ptr %[[X]]
+
+// OGCG: define {{.*}} i32 @_Z4testv()
+// OGCG:   %[[X:.*]] = alloca ptr
+// OGCG:   %[[TEMP_SLOT:.*]] = alloca i32
+// OGCG:   %[[TEMP_VALUE:.*]] = call noundef i32 @_Z8make_intv()
+// OGCG:   store i32 %[[TEMP_VALUE]], ptr %[[TEMP_SLOT]]
+// OGCG:   store ptr %[[TEMP_SLOT]], ptr %[[X]]
+
+int test_scoped() {
+  int x = make_int();
+  {
+    const int &y = make_int();
+    x = y;
+  }
+  return x;
+}
+
+//      CIR: cir.func {{.*}} @_Z11test_scopedv()
+//      CIR:   %[[X:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["x", init]
+//      CIR:   cir.scope {
+// CIR-NEXT:     %[[TEMP_SLOT:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["ref.tmp0", init]
+// CIR-NEXT:     %[[Y_ADDR:.*]] = cir.alloca !cir.ptr<!s32i>, !cir.ptr<!cir.ptr<!s32i>>, ["y", init, const]
+// CIR-NEXT:     cir.scope {
+// CIR-NEXT:       %[[TEMP_VALUE:.*]] = cir.call @_Z8make_intv() : () -> !s32i
+// CIR-NEXT:       cir.store{{.*}} %[[TEMP_VALUE]], %[[TEMP_SLOT]] : !s32i, !cir.ptr<!s32i>
+// CIR-NEXT:     }
+// CIR-NEXT:     cir.store{{.*}} %[[TEMP_SLOT]], %[[Y_ADDR]] : !cir.ptr<!s32i>, !cir.ptr<!cir.ptr<!s32i>>
+// CIR-NEXT:     %[[Y_REF:.*]] = cir.load %[[Y_ADDR]] : !cir.ptr<!cir.ptr<!s32i>>, !cir.ptr<!s32i>
+// CIR-NEXT:     %[[Y_VALUE:.*]] = cir.load{{.*}} %[[Y_REF]] : !cir.ptr<!s32i>, !s32i
+// CIR-NEXT:     cir.store{{.*}} %[[Y_VALUE]], %[[X]] : !s32i, !cir.ptr<!s32i>
+// CIR-NEXT:   }
+
+// LLVM: define {{.*}} i32 @_Z11test_scopedv()
+// LLVM:   %[[TEMP_SLOT:.*]] = alloca i32
+// LLVM:   %[[Y_ADDR:.*]] = alloca ptr
+// LLVM:   %[[RETVAL:.*]] = alloca i32
+// LLVM:   %[[X:.*]] = alloca i32
+// LLVM:   %[[TEMP_VALUE1:.*]] = call i32 @_Z8make_intv()
+// LLVM:   store i32 %[[TEMP_VALUE1]], ptr %[[X]]
+// LLVM:   br label %[[SCOPE_LABEL:.*]]
+// LLVM: [[SCOPE_LABEL]]:
+// LLVM:   br label %[[INNER_SCOPE_LABEL:.*]]
+// LLVM: [[INNER_SCOPE_LABEL]]:
+// LLVM:   %[[TEMP_VALUE2:.*]] = call i32 @_Z8make_intv()
+// LLVM:   store i32 %[[TEMP_VALUE2]], ptr %[[TEMP_SLOT]]
+// LLVM:   br label %[[INNER_SCOPE_END_LABEL:.*]]
+// LLVM: [[INNER_SCOPE_END_LABEL]]:
+// LLVM:   store ptr %[[TEMP_SLOT]], ptr %[[Y_ADDR]]
+// LLVM:   %[[Y_REF:.*]] = load ptr, ptr %[[Y_ADDR]]
+// LLVM:   %[[Y_VALUE:.*]] = load i32, ptr %[[Y_REF]]
+// LLVM:   store i32 %[[Y_VALUE]], ptr %[[X]]
+
+// OGCG: define {{.*}} i32 @_Z11test_scopedv()
+// OGCG:   %[[X:.*]] = alloca i32
+// OGCG:   %[[Y_ADDR:.*]] = alloca ptr
+// OGCG:   %[[TEMP_SLOT:.*]] = alloca i32
+// OGCG:   %[[TEMP_VALUE1:.*]] = call noundef i32 @_Z8make_intv()
+// OGCG:   store i32 %[[TEMP_VALUE1]], ptr %[[X]]
+// OGCG:   %[[TEMP_VALUE2:.*]] = call noundef i32 @_Z8make_intv()
+// OGCG:   store i32 %[[TEMP_VALUE2]], ptr %[[TEMP_SLOT]]
+// OGCG:   store ptr %[[TEMP_SLOT]], ptr %[[Y_ADDR]]
+// OGCG:   %[[Y_REF:.*]] = load ptr, ptr %[[Y_ADDR]]
+// OGCG:   %[[Y_VALUE:.*]] = load i32, ptr %[[Y_REF]]
+// OGCG:   store i32 %[[Y_VALUE]], ptr %[[X]]

[erichkeane]

I dont see any statement expression tests?

[andykaylor]

Oof! This is ugly. I was struggling to find a test case that would get us here in a situation that actually needed to do a cleanup. I just dug through the revision history to find the test case that was added when the corresponding code was added in classic codegen. It turns out the incubator (and hence this PR) doesn't handle that case correctly.

https://godbolt.org/z/36nEss113

This code should be forcing a cleanup with a function variant that accepts a list of values the needed to be re-loaded in the cleanup handler. That variant exists in the incubator but it doesn't handle the reloading of the variables. I'm not sure what it will take to implement it, but we're definitely not ready for that yet. It requires a level of cleanup branch handling that we aren't prepared for.

I think I need to just add a missing feature here.

[erichkeane]

I'm just glad you figured this out before I'd have to figure out how the heck to write a statement expression again :D

[erichkeane]

Can we explain the 'else' here? It is odd to me that 'simple' is the only thing that would require work.

[andykaylor]

That's what the FIXME below is about (carried over from classic codegen). I can't think of a way to hit the else case. Nothing in the current clang lit tests goes to the 'else' case.

[erichkeane]

Hrmph, ok then. CodeGen constantly leaves me in a state of disgruntled wonder.