When lowering "cond ? X : Y" we do some safety checks to see whether we can
instead emit both X and Y and use a llvm select instruction to choose
between them. This code is insufficiently safe, and introducing loads into
a program that didn't load is a bad idea. For example, it could be TLS. It
could be a non-volatile auto in another function that isn't the current
function (think lambdas).
Don't do this here. LLVM knows how to do this properly.
Patch attached, please review!
Nick
Index: test/CodeGen/catch-undef-behavior.c
===================================================================
--- test/CodeGen/catch-undef-behavior.c (revision 194042)
+++ test/CodeGen/catch-undef-behavior.c (working copy)
@@ -432,6 +432,14 @@
return *p;
}
+_Bool GetOptionalBool(_Bool *value);
+_Bool GetBool(_Bool default_value) {
+ // CHECK-LABEL: @GetBool
+ // CHECK-NOT: select
+ _Bool value;
+ return GetOptionalBool(&value) ? value : default_value;
+}
+
// CHECK: ![[WEIGHT_MD]] = metadata !{metadata !"branch_weights", i32 1048575, i32 1}
// CHECK-TRAP: attributes [[NR_NUW]] = { noreturn nounwind }
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp (revision 194042)
+++ lib/CodeGen/CGExprScalar.cpp (working copy)
@@ -3023,22 +3023,15 @@
/// flow into selects in some cases.
static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E,
CodeGenFunction &CGF) {
- E = E->IgnoreParens();
-
// Anything that is an integer or floating point constant is fine.
- if (E->isEvaluatable(CGF.getContext()))
- return true;
+ return E->IgnoreParens()->isEvaluatable(CGF.getContext());
- // Non-volatile automatic variables too, to get "cond ? X : Y" where
- // X and Y are local variables.
- if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
- if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()))
- if (VD->hasLocalStorage() && !(CGF.getContext()
- .getCanonicalType(VD->getType())
- .isVolatileQualified()))
- return true;
-
- return false;
+ // Even non-volatile automatic variables can't be evaluated unconditionally.
+ // Referencing a thread_local may cause non-trivial initialization work to
+ // occur. If we're inside a lambda and one of the variables is from the scope
+ // outside the lambda, that function may have returned already. Reading its
+ // locals is a bad idea. Also, these reads may introduce races there didn't
+ // exist in the source-level program.
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
