]> granicus.if.org Git - clang/commitdiff
Don't try to spill static allocas when emitting expr cleanups with branches
authorReid Kleckner <rnk@google.com>
Wed, 31 May 2017 19:59:41 +0000 (19:59 +0000)
committerReid Kleckner <rnk@google.com>
Wed, 31 May 2017 19:59:41 +0000 (19:59 +0000)
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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@304335 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/CGCleanup.cpp
test/CodeGenCXX/stmtexpr.cpp

index e8bcf0a3ac56409a963dbf42f20528fd5af2cdec..b5453bc11e305ff7dd5e10619f5f4cb51e8437bd 100644 (file)
@@ -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");
 
index 5885a1663e635760fb3142843e8342620511a644..5bd9908d6c2576a8a797ac6f1b56872ebfbc9910 100644 (file)
@@ -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 cond) {
 // 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.