Author: rsmith Date: Fri Sep 16 18:30:39 2016 New Revision: 281797 URL: http://llvm.org/viewvc/llvm-project?rev=281797&view=rev Log: 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/trunk/lib/CodeGen/CGStmt.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=281797&r1=281796&r2=281797&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original) +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Fri Sep 16 18:30:39 2016 @@ -1329,6 +1329,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 @@ -1338,7 +1342,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; @@ -1374,11 +1378,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: @@ -1396,7 +1408,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/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=281797&r1=281796&r2=281797&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Sep 16 18:30:39 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" @@ -1168,6 +1169,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/trunk/lib/CodeGen/CodeGenFunction.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=281797&r1=281796&r2=281797&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Sep 16 18:30:39 2016 @@ -3168,6 +3168,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/trunk/test/CodeGenCXX/switch-case-folding-2.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp?rev=281797&r1=281796&r2=281797&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp (original) +++ cfe/trunk/test/CodeGenCXX/switch-case-folding-2.cpp Fri Sep 16 18:30:39 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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits