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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
