[clang] [CIR][CodeGen] Eliminate unnecessary __retval alloca for simple returns (PR #186320)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Jie Zhang (zj040045)

<details>
<summary>Changes</summary>

Remove redundant load-store, and clear unnecessary __retval alloca in the end. A test is also added to verify the behavior.

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


3 Files Affected:

- (modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+4) 
- (modified) clang/lib/CIR/CodeGen/CIRGenStmt.cpp (+38-7) 
- (added) clang/test/CIR/CodeGen/retval.c (+40) 


``````````diff
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 71a647e37ea52..5d3aedd8ecccb 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -815,6 +815,10 @@ cir::FuncOp CIRGenFunction::generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
   }
 
   eraseEmptyAndUnusedBlocks(fn);
+
+  if (fnRetAlloca && fnRetAlloca->use_empty())
+    fnRetAlloca->getDefiningOp()->erase();
+
   return fn;
 }
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index a33a0b8bdd50c..871a1ebed0b2b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -585,6 +585,31 @@ mlir::LogicalResult CIRGenFunction::emitDeclStmt(const DeclStmt &s) {
   return mlir::success();
 }
 
+static cir::StoreOp findDominatingStoreToReturnValue(CIRGenFunction &cgf) {
+  mlir::Block *currentBlock = cgf.getBuilder().getInsertionBlock();
+  if (!currentBlock || currentBlock->empty())
+    return nullptr;
+
+  if (!cgf.fnRetAlloca)
+    return nullptr;
+
+  mlir::Value retAlloca = *cgf.fnRetAlloca;
+
+  for (auto &op : llvm::reverse(*currentBlock)) {
+    if (auto storeOp = dyn_cast<cir::StoreOp>(op)) {
+      if (storeOp.getAddr() == retAlloca) {
+        return storeOp;
+      }
+      return nullptr;
+    }
+
+    if (op.hasTrait<mlir::OpTrait::IsTerminator>() || isa<cir::CallOp>(op)) {
+      return nullptr;
+    }
+  }
+  return nullptr;
+}
+
 mlir::LogicalResult CIRGenFunction::emitReturnStmt(const ReturnStmt &s) {
   mlir::Location loc = getLoc(s.getSourceRange());
   const Expr *rv = s.getRetValue();
@@ -677,15 +702,21 @@ mlir::LogicalResult CIRGenFunction::emitReturnStmt(const ReturnStmt &s) {
   // a shared return block. Because CIR handles branching through cleanups
   // during the CFG flattening phase, we can just emit the return statement
   // directly.
-  // TODO(cir): Eliminate this redundant load and the store above when we can.
   if (fnRetAlloca) {
-    // Load the value from `__retval` and return it via the `cir.return` op.
-    cir::AllocaOp retAlloca =
-        mlir::cast<cir::AllocaOp>(fnRetAlloca->getDefiningOp());
-    auto value = cir::LoadOp::create(builder, loc, retAlloca.getAllocaType(),
-                                     *fnRetAlloca);
+    mlir::Value returnValue;
+    if (cir::StoreOp storeOp = findDominatingStoreToReturnValue(*this)) {
+      returnValue = storeOp.getValue();
+      storeOp.erase();
+    } else {
+      // Load the value from `__retval` and return it via the `cir.return` op.
+      cir::AllocaOp retAlloca =
+          mlir::cast<cir::AllocaOp>(fnRetAlloca->getDefiningOp());
+      auto loadOp = cir::LoadOp::create(builder, loc, retAlloca.getAllocaType(),
+                                        *fnRetAlloca);
+      returnValue = loadOp.getResult();
+    }
 
-    cir::ReturnOp::create(builder, loc, {value});
+    cir::ReturnOp::create(builder, loc, {returnValue});
   } else {
     cir::ReturnOp::create(builder, loc);
   }
diff --git a/clang/test/CIR/CodeGen/retval.c b/clang/test/CIR/CodeGen/retval.c
new file mode 100644
index 0000000000000..454082338fc8f
--- /dev/null
+++ b/clang/test/CIR/CodeGen/retval.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s
+
+// Simple scalar return: __retval should be eliminated.
+int add(int a, int b) { return a + b; }
+
+// CHECK-LABEL: cir.func{{.*}} @add(
+// CHECK-NOT:     ["__retval"]
+// CHECK:         cir.return
+// CHECK:       }
+
+// Void return: no __retval at all.
+void noop(void) {}
+
+// CHECK-LABEL: cir.func{{.*}} @noop(
+// CHECK-NOT:     ["__retval"]
+// CHECK:         cir.return
+// CHECK:       }
+
+int select_val(int a, int b, int c) {
+  if (c) return a;
+  return b;
+}
+
+// CHECK-LABEL: cir.func{{.*}} @select_val(
+// CHECK-NOT:     ["__retval"]
+// CHECK:         cir.return
+// CHECK:       }
+
+typedef struct { int x; int y; } Pair;
+Pair make_pair(int a, int b) {
+  Pair p = {a, b};
+  return p;
+}
+
+// CHECK-LABEL: cir.func{{.*}} @make_pair(
+// CHECK:         cir.alloca !rec_Pair, !cir.ptr<!rec_Pair>, ["__retval", init]
+// CHECK:         cir.load %{{.+}} : !cir.ptr<!rec_Pair>, !rec_Pair
+// CHECK:         cir.return
+// CHECK:       }

``````````

</details>


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