From 80311fbec8c9f07dead8cd9ccf6774957cf4fa32 Mon Sep 17 00:00:00 2001
From: Vinicius Couto Espindola <vini.couto.e@gmail.com>
Date: Tue, 14 Nov 2023 21:45:35 -0300
Subject: [PATCH] [CIR][CIRGen][NFC] Return scope result in compound stmt
 builders

Instead of returning a boolean indicating whether the statement was
handled, returns the ReturnExpr of the statement if there is one. It
also adds some extra bookkeeping to ensure that the result is returned
when needed. This allows for better support of GCC's `ExprStmt`
extension.

The logical result was not used: it was handled but it would never fail.
Any errors within builders should likely be handled with asserts and
unreachables since they imply a programmer's error in the code.

[ghstack-poisoned]
---
 clang/lib/CIR/CodeGen/CIRGenFunction.cpp |  3 +-
 clang/lib/CIR/CodeGen/CIRGenFunction.h   | 10 ++++---
 clang/lib/CIR/CodeGen/CIRGenStmt.cpp     | 36 ++++++++++++------------
 3 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index a83ec5a5c1bd..85c6891ec454 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -24,6 +24,7 @@
 #include "clang/Frontend/FrontendDiagnostic.h"
 
 #include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Support/LogicalResult.h"
 
 using namespace cir;
 using namespace clang;
@@ -1102,7 +1103,7 @@ mlir::LogicalResult CIRGenFunction::buildFunctionBody(const clang::Stmt *Body) {
 
   auto result = mlir::LogicalResult::success();
   if (const CompoundStmt *S = dyn_cast<CompoundStmt>(Body))
-    result = buildCompoundStmtWithoutScope(*S);
+    buildCompoundStmtWithoutScope(*S);
   else
     result = buildStmt(Body, /*useCurrentScope*/ true);
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 3b3a63a0d3e8..d43355d8a59d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -1006,11 +1006,13 @@ class CIRGenFunction : public CIRGenTypeCache {
                        bool IsFnTryBlock = false);
   void exitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false);
 
-  mlir::LogicalResult buildCompoundStmt(const clang::CompoundStmt &S);
-
-  mlir::LogicalResult
-  buildCompoundStmtWithoutScope(const clang::CompoundStmt &S);
+  Address buildCompoundStmt(const clang::CompoundStmt &S, bool getLast = false,
+                            AggValueSlot slot = AggValueSlot::ignored());
 
+  Address
+  buildCompoundStmtWithoutScope(const clang::CompoundStmt &S,
+                                bool getLast = false,
+                                AggValueSlot slot = AggValueSlot::ignored());
   GlobalDecl CurSEHParent;
   bool currentFunctionUsesSEHTry() const { return !!CurSEHParent; }
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index c3926ea99bf3..46aea77418ba 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -10,30 +10,27 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Address.h"
 #include "CIRGenFunction.h"
+#include "mlir/IR/Value.h"
 
 using namespace cir;
 using namespace clang;
 using namespace mlir::cir;
 
-mlir::LogicalResult
-CIRGenFunction::buildCompoundStmtWithoutScope(const CompoundStmt &S) {
+Address CIRGenFunction::buildCompoundStmtWithoutScope(const CompoundStmt &S,
+                                                      bool getLast,
+                                                      AggValueSlot slot) {
   for (auto *CurStmt : S.body())
     if (buildStmt(CurStmt, /*useCurrentScope=*/false).failed())
-      return mlir::failure();
+      return Address::invalid();
 
-  return mlir::success();
+  return Address::invalid();
 }
 
-mlir::LogicalResult CIRGenFunction::buildCompoundStmt(const CompoundStmt &S) {
-  mlir::LogicalResult res = mlir::success();
-
-  auto compoundStmtBuilder = [&]() -> mlir::LogicalResult {
-    if (buildCompoundStmtWithoutScope(S).failed())
-      return mlir::failure();
-
-    return mlir::success();
-  };
+Address CIRGenFunction::buildCompoundStmt(const CompoundStmt &S, bool getLast,
+                                          AggValueSlot slot) {
+  Address retAlloca = Address::invalid();
 
   // Add local scope to track new declared variables.
   SymTableScopeTy varScope(symbolTable);
@@ -43,10 +40,10 @@ mlir::LogicalResult CIRGenFunction::buildCompoundStmt(const CompoundStmt &S) {
       [&](mlir::OpBuilder &b, mlir::Location loc) {
         LexicalScopeContext lexScope{loc, builder.getInsertionBlock()};
         LexicalScopeGuard lexScopeGuard{*this, &lexScope};
-        res = compoundStmtBuilder();
+        retAlloca = buildCompoundStmtWithoutScope(S);
       });
 
-  return res;
+  return retAlloca;
 }
 
 void CIRGenFunction::buildStopPoint(const Stmt *S) {
@@ -258,9 +255,12 @@ mlir::LogicalResult CIRGenFunction::buildSimpleStmt(const Stmt *S,
   case Stmt::DeclStmtClass:
     return buildDeclStmt(cast<DeclStmt>(*S));
   case Stmt::CompoundStmtClass:
-    return useCurrentScope
-               ? buildCompoundStmtWithoutScope(cast<CompoundStmt>(*S))
-               : buildCompoundStmt(cast<CompoundStmt>(*S));
+    // FIXME(cir): This is a form of early optimization. Perhaps we should
+    // always emit a `cir.scope` if one exists in the source code, then
+    // canonicalize it away with the merge-cleanup pass.
+    useCurrentScope ? buildCompoundStmtWithoutScope(cast<CompoundStmt>(*S))
+                    : buildCompoundStmt(cast<CompoundStmt>(*S));
+    break;
   case Stmt::ReturnStmtClass:
     return buildReturnStmt(cast<ReturnStmt>(*S));
   case Stmt::GotoStmtClass: