r304335 - Don't try to spill static allocas when emitting expr cleanups with branches

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed May 31 14:59:41 2017
New Revision: 304335

URL: http://llvm.org/viewvc/llvm-project?rev=304335&view=rev
Log:
Don't try to spill static allocas when emitting expr cleanups with branches

Credit goes to Gor Nishanov for putting together the fix in
https://reviews.llvm.org/D33733!

This patch is essentially me patching it locally and writing some test
cases to convince myself that it was necessary for GNU statement
expressions with branches as well as coroutines. I'll ask Gor to land
his patch with just the coroutines test.

During LValue expression evaluation, references can be bound to
anything, really: call results, aggregate temporaries, local variables,
global variables, or indirect arguments. We really only want to spill
instructions that were emitted as part of expression evaluation, and
static allocas are not that.

Modified:
    cfe/trunk/lib/CodeGen/CGCleanup.cpp
    cfe/trunk/test/CodeGenCXX/stmtexpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=304335&r1=304334&r2=304335&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Wed May 31 14:59:41 2017
@@ -448,6 +448,13 @@ void CodeGenFunction::PopCleanupBlocks(
     auto *Inst = dyn_cast_or_null<llvm::Instruction>(*ReloadedValue);
     if (!Inst)
       continue;
+
+    // Don't spill static allocas, they dominate all cleanups. These are created
+    // by binding a reference to a local variable or temporary.
+    auto *AI = dyn_cast<llvm::AllocaInst>(Inst);
+    if (AI && AI->isStaticAlloca())
+      continue;
+
     Address Tmp =
         CreateDefaultAlignTempAlloca(Inst->getType(), "tmp.exprcleanup");
 

Modified: cfe/trunk/test/CodeGenCXX/stmtexpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/stmtexpr.cpp?rev=304335&r1=304334&r2=304335&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/stmtexpr.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/stmtexpr.cpp Wed May 31 14:59:41 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -Wno-unused-value -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -Wno-unused-value -triple i686-linux-gnu -emit-llvm -o - %s | FileCheck %s
 // rdar: //8540501
 extern "C" int printf(...);
 extern "C" void abort();
@@ -139,6 +139,34 @@ extern "C" int cleanup_exit_lvalue(bool
 // CHECK: %[[v:[^ ]*]] = load i32*, i32** %[[tmp]]
 // CHECK-NEXT: store i32* %[[v]], i32** %r
 
+// Bind the reference to a byval argument. It is not an instruction or Constant,
+// so it's a bit of a corner case.
+struct ByVal { int x[3]; };
+extern "C" int cleanup_exit_lvalue_byval(bool cond, ByVal arg) {
+  ByVal &r = (A(1), ({ if (cond) return 0; (void)ByVal(); }), arg);
+  return r.x[0];
+}
+// CHECK-LABEL: define{{.*}} i32 @cleanup_exit_lvalue_byval({{.*}}, %struct.ByVal* byval align 4 %arg)
+// CHECK: call {{.*}} @_ZN1AC1Ei
+// CHECK: call {{.*}} @_ZN1AD1Ev
+// CHECK: switch
+// CHECK: store %struct.ByVal* %arg, %struct.ByVal** %r
+
+// Bind the reference to a local variable. We don't need to spill it. Binding a
+// reference to it doesn't generate any instructions.
+extern "C" int cleanup_exit_lvalue_local(bool cond) {
+  int local = 42;
+  int &r = (A(1), ({ if (cond) return 0; (void)0; }), local);
+  return r;
+}
+// CHECK-LABEL: define{{.*}} i32 @cleanup_exit_lvalue_local({{.*}})
+// CHECK: %local = alloca i32
+// CHECK: store i32 42, i32* %local
+// CHECK: call {{.*}} @_ZN1AC1Ei
+// CHECK-NOT: store i32* %local
+// CHECK: call {{.*}} @_ZN1AD1Ev
+// CHECK: switch
+// CHECK: store i32* %local, i32** %r, align 4
 
 // We handle ExprWithCleanups for complex evaluation type separately, and it had
 // the same bug.