[llvm-branch-commits] [cfe-branch] r287790 - Merging r281797:
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Nov 23 10:21:42 PST 2016
Author: tstellar
Date: Wed Nov 23 12:21:42 2016
New Revision: 287790
URL: http://llvm.org/viewvc/llvm-project?rev=287790&view=rev
Log:
Merging r281797:
------------------------------------------------------------------------
r281797 | richard-llvm | 2016-09-16 16:30:39 -0700 (Fri, 16 Sep 2016) | 10 lines
Fix a couple of wrong-code bugs in switch-on-constant optimization:
* recurse through intermediate LabelStmts and AttributedStmts when checking
whether a statement inside a switch declares a variable
* if the end of a compound statement is reachable from the chosen case label,
and the compound statement contains a variable declaration, it's not valid
to just emit the contents of the compound statement -- we must emit the
statement itself or we lose the scope (and thus end lifetimes at the wrong
point)
------------------------------------------------------------------------
Modified:
cfe/branches/release_39/lib/CodeGen/CGStmt.cpp
cfe/branches/release_39/lib/CodeGen/CodeGenFunction.cpp
cfe/branches/release_39/lib/CodeGen/CodeGenFunction.h
cfe/branches/release_39/test/CodeGenCXX/switch-case-folding-2.cpp
Modified: cfe/branches/release_39/lib/CodeGen/CGStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_39/lib/CodeGen/CGStmt.cpp?rev=287790&r1=287789&r2=287790&view=diff
==============================================================================
--- cfe/branches/release_39/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/branches/release_39/lib/CodeGen/CGStmt.cpp Wed Nov 23 12:21:42 2016
@@ -1323,6 +1323,10 @@ static CSFC_Result CollectStatementsForC
// Handle this as two cases: we might be looking for the SwitchCase (if so
// the skipped statements must be skippable) or we might already have it.
CompoundStmt::const_body_iterator I = CS->body_begin(), E = CS->body_end();
+ bool StartedInLiveCode = FoundCase;
+ unsigned StartSize = ResultStmts.size();
+
+ // If we've not found the case yet, scan through looking for it.
if (Case) {
// Keep track of whether we see a skipped declaration. The code could be
// using the declaration even if it is skipped, so we can't optimize out
@@ -1332,7 +1336,7 @@ static CSFC_Result CollectStatementsForC
// If we're looking for the case, just see if we can skip each of the
// substatements.
for (; Case && I != E; ++I) {
- HadSkippedDecl |= isa<DeclStmt>(*I);
+ HadSkippedDecl |= CodeGenFunction::mightAddDeclToScope(*I);
switch (CollectStatementsForCase(*I, Case, FoundCase, ResultStmts)) {
case CSFC_Failure: return CSFC_Failure;
@@ -1368,11 +1372,19 @@ static CSFC_Result CollectStatementsForC
break;
}
}
+
+ if (!FoundCase)
+ return CSFC_Success;
+
+ assert(!HadSkippedDecl && "fallthrough after skipping decl");
}
// If we have statements in our range, then we know that the statements are
// live and need to be added to the set of statements we're tracking.
+ bool AnyDecls = false;
for (; I != E; ++I) {
+ AnyDecls |= CodeGenFunction::mightAddDeclToScope(*I);
+
switch (CollectStatementsForCase(*I, nullptr, FoundCase, ResultStmts)) {
case CSFC_Failure: return CSFC_Failure;
case CSFC_FallThrough:
@@ -1390,7 +1402,24 @@ static CSFC_Result CollectStatementsForC
}
}
- return Case ? CSFC_Success : CSFC_FallThrough;
+ // If we're about to fall out of a scope without hitting a 'break;', we
+ // can't perform the optimization if there were any decls in that scope
+ // (we'd lose their end-of-lifetime).
+ if (AnyDecls) {
+ // If the entire compound statement was live, there's one more thing we
+ // can try before giving up: emit the whole thing as a single statement.
+ // We can do that unless the statement contains a 'break;'.
+ // FIXME: Such a break must be at the end of a construct within this one.
+ // We could emit this by just ignoring the BreakStmts entirely.
+ if (StartedInLiveCode && !CodeGenFunction::containsBreak(S)) {
+ ResultStmts.resize(StartSize);
+ ResultStmts.push_back(S);
+ } else {
+ return CSFC_Failure;
+ }
+ }
+
+ return CSFC_FallThrough;
}
// Okay, this is some other statement that we don't handle explicitly, like a
Modified: cfe/branches/release_39/lib/CodeGen/CodeGenFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_39/lib/CodeGen/CodeGenFunction.cpp?rev=287790&r1=287789&r2=287790&view=diff
==============================================================================
--- cfe/branches/release_39/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/branches/release_39/lib/CodeGen/CodeGenFunction.cpp Wed Nov 23 12:21:42 2016
@@ -25,6 +25,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/StmtCXX.h"
+#include "clang/AST/StmtObjC.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/CodeGen/CGFunctionInfo.h"
@@ -1159,6 +1160,28 @@ bool CodeGenFunction::containsBreak(cons
return false;
}
+bool CodeGenFunction::mightAddDeclToScope(const Stmt *S) {
+ if (!S) return false;
+
+ // Some statement kinds add a scope and thus never add a decl to the current
+ // scope. Note, this list is longer than the list of statements that might
+ // have an unscoped decl nested within them, but this way is conservatively
+ // correct even if more statement kinds are added.
+ if (isa<IfStmt>(S) || isa<SwitchStmt>(S) || isa<WhileStmt>(S) ||
+ isa<DoStmt>(S) || isa<ForStmt>(S) || isa<CompoundStmt>(S) ||
+ isa<CXXForRangeStmt>(S) || isa<CXXTryStmt>(S) ||
+ isa<ObjCForCollectionStmt>(S) || isa<ObjCAtTryStmt>(S))
+ return false;
+
+ if (isa<DeclStmt>(S))
+ return true;
+
+ for (const Stmt *SubStmt : S->children())
+ if (mightAddDeclToScope(SubStmt))
+ return true;
+
+ return false;
+}
/// ConstantFoldsToSimpleInteger - If the specified expression does not fold
/// to a constant, or if it does but contains a label, return false. If it
Modified: cfe/branches/release_39/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_39/lib/CodeGen/CodeGenFunction.h?rev=287790&r1=287789&r2=287790&view=diff
==============================================================================
--- cfe/branches/release_39/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/branches/release_39/lib/CodeGen/CodeGenFunction.h Wed Nov 23 12:21:42 2016
@@ -3192,6 +3192,10 @@ public:
/// If the statement (recursively) contains a switch or loop with a break
/// inside of it, this is fine.
static bool containsBreak(const Stmt *S);
+
+ /// Determine if the given statement might introduce a declaration into the
+ /// current scope, by being a (possibly-labelled) DeclStmt.
+ static bool mightAddDeclToScope(const Stmt *S);
/// ConstantFoldsToSimpleInteger - If the specified expression does not fold
/// to a constant, or if it does but contains a label, return false. If it
Modified: cfe/branches/release_39/test/CodeGenCXX/switch-case-folding-2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_39/test/CodeGenCXX/switch-case-folding-2.cpp?rev=287790&r1=287789&r2=287790&view=diff
==============================================================================
--- cfe/branches/release_39/test/CodeGenCXX/switch-case-folding-2.cpp (original)
+++ cfe/branches/release_39/test/CodeGenCXX/switch-case-folding-2.cpp Wed Nov 23 12:21:42 2016
@@ -1,12 +1,14 @@
// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s
-// CHECK that we don't crash.
extern int printf(const char*, ...);
+
+// CHECK-LABEL: @_Z4testi(
int test(int val){
switch (val) {
case 4:
do {
switch (6) {
+ // CHECK: call i32 (i8*, ...) @_Z6printfPKcz
case 6: do { case 5: printf("bad\n"); } while (0);
};
} while (0);
@@ -18,6 +20,7 @@ int main(void) {
return test(5);
}
+// CHECK-LABEL: @_Z10other_testv(
void other_test() {
switch(0) {
case 0:
@@ -27,4 +30,79 @@ void other_test() {
}
}
-// CHECK: call i32 (i8*, ...) @_Z6printfPKcz
+struct X { X(); ~X(); };
+
+void dont_call();
+void foo();
+
+// CHECK-LABEL: @_Z13nested_scopesv(
+void nested_scopes() {
+ switch (1) {
+ case 0:
+ // CHECK-NOT: @_Z9dont_callv(
+ dont_call();
+ break;
+
+ default:
+ // CHECK: call {{.*}} @_ZN1XC1Ev(
+ // CHECK: call {{.*}} @_Z3foov(
+ // CHECK-NOT: call {{.*}} @_Z3foov(
+ // CHECK: call {{.*}} @_ZN1XD1Ev(
+ { X x; foo(); }
+
+ // CHECK: call {{.*}} @_ZN1XC1Ev(
+ // CHECK: call {{.*}} @_Z3foov(
+ // CHECK: call {{.*}} @_ZN1XD1Ev(
+ { X x; foo(); }
+
+ // CHECK: call {{.*}} @_ZN1XC1Ev(
+ // CHECK: call {{.*}} @_Z3foov(
+ // CHECK: call {{.*}} @_ZN1XD1Ev(
+ { X x; foo(); }
+ break;
+ }
+}
+
+// CHECK-LABEL: @_Z17scope_fallthroughv(
+void scope_fallthrough() {
+ switch (1) {
+ // CHECK: call {{.*}} @_ZN1XC1Ev(
+ // CHECK-NOT: call {{.*}} @_Z3foov(
+ // CHECK: call {{.*}} @_ZN1XD1Ev(
+ { default: X x; }
+ // CHECK: call {{.*}} @_Z3foov(
+ foo();
+ break;
+ }
+}
+
+// CHECK-LABEL: @_Z12hidden_breakb(
+void hidden_break(bool b) {
+ switch (1) {
+ default:
+ // CHECK: br
+ if (b)
+ break;
+ // CHECK: call {{.*}} @_Z3foov(
+ foo();
+ break;
+ }
+}
+
+// CHECK-LABEL: @_Z10hidden_varv(
+int hidden_var() {
+ switch (1) {
+ // CHECK: %[[N:.*]] = alloca i32
+ case 0: int n;
+ // CHECK: store i32 0, i32* %[[N]]
+ // CHECK: load i32, i32* %[[N]]
+ // CHECK: ret
+ default: n = 0; return n;
+ }
+}
+
+// CHECK-LABEL: @_Z13case_in_labelv(
+void case_in_label() {
+ // CHECK: br label %
+ switch (1) case 1: foo: case 0: goto foo;
+}
More information about the llvm-branch-commits
mailing list