<div dir="ltr"><div dir="ltr">On Fri, 12 Jun 2020 at 09:17, Erich Keane via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Erich Keane<br>
Date: 2020-06-12T09:16:43-07:00<br>
New Revision: 1eddce4177cfddc86d4696b758904443b0b4f193<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/1eddce4177cfddc86d4696b758904443b0b4f193" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/1eddce4177cfddc86d4696b758904443b0b4f193</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/1eddce4177cfddc86d4696b758904443b0b4f193.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/1eddce4177cfddc86d4696b758904443b0b4f193.diff</a><br>
<br>
LOG: Fix non-determinism issue with implicit lambda captures.<br>
<br>
We were using llvm::SmallPtrSet for our ODR-use set which was also used<br>
for instantiating the implicit lambda captures. The order in which the<br>
captures are added depends on this, so the lambda's layout ended up<br>
changing. The test just uses floats, but this was noticed with other<br>
types as well.<br>
<br>
This test replaces the short-lived SmallPtrSet (it lasts only for an<br>
expression, which, though is a long time for lambdas, is at least not<br>
forever) with a SmallSetVector.<br></blockquote><div><br></div><div>Wow, that's bad. Nice catch.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Added: <br>
clang/test/CodeGenCXX/lambda-deterministic-captures.cpp<br>
<br>
Modified: <br>
clang/include/clang/Sema/Sema.h<br>
clang/lib/Sema/SemaExpr.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h<br>
index 266192087d35..a3f4313c0d65 100644<br>
--- a/clang/include/clang/Sema/Sema.h<br>
+++ b/clang/include/clang/Sema/Sema.h<br>
@@ -627,7 +627,7 @@ class Sema final {<br>
/// we won't know until all lvalue-to-rvalue and discarded value conversions<br>
/// have been applied to all subexpressions of the enclosing full expression.<br>
/// This is cleared at the end of each full expression.<br>
- using MaybeODRUseExprSet = llvm::SmallPtrSet<Expr *, 2>;<br>
+ using MaybeODRUseExprSet = llvm::SmallSetVector<Expr *, 2>;<br>
MaybeODRUseExprSet MaybeODRUseExprs;<br>
<br>
std::unique_ptr<sema::FunctionScopeInfo> CachedFunctionScope;<br>
<br>
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp<br>
index 6477979e92fe..6965acdb6163 100644<br>
--- a/clang/lib/Sema/SemaExpr.cpp<br>
+++ b/clang/lib/Sema/SemaExpr.cpp<br>
@@ -17553,7 +17553,7 @@ static ExprResult rebuildPotentialResultsAsNonOdrUsed(Sema &S, Expr *E,<br>
<br>
// Mark that this expression does not constitute an odr-use.<br>
auto MarkNotOdrUsed = [&] {<br>
- S.MaybeODRUseExprs.erase(E);<br>
+ S.MaybeODRUseExprs.remove(E);<br>
if (LambdaScopeInfo *LSI = S.getCurLambda())<br>
LSI->markVariableExprAsNonODRUsed(E);<br>
};<br>
<br>
diff --git a/clang/test/CodeGenCXX/lambda-deterministic-captures.cpp b/clang/test/CodeGenCXX/lambda-deterministic-captures.cpp<br>
new file mode 100644<br>
index 000000000000..d025a431d5c5<br>
--- /dev/null<br>
+++ b/clang/test/CodeGenCXX/lambda-deterministic-captures.cpp<br>
@@ -0,0 +1,33 @@<br>
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm --std=c++17 %s -o - | FileCheck %s<br>
+<br>
+struct stream {<br>
+ friend const stream &operator<<(const stream &, const float &);<br>
+};<br>
+<br>
+void foo() {<br>
+ constexpr float f_zero = 0.0f;<br>
+ constexpr float f_one = 1.0f;<br>
+ constexpr float f_two = 2.0f;<br>
+<br>
+ stream s;<br>
+ [=]() {<br>
+ s << f_zero << f_one << f_two;<br>
+ }();<br>
+}<br>
+<br>
+// CHECK: define void @_Z3foov<br>
+// CHECK: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 0<br>
+// CHECK-NEXT: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 1<br>
+// CHECK-NEXT: store float 0.000<br>
+// CHECK-NEXT: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 2<br>
+// CHECK-NEXT: store float 1.000<br>
+// CHECK-NEXT: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 3<br>
+// CHECK-NEXT: store float 2.000<br>
+<br>
+// The lambda body. Reverse iteration when the captures aren't deterministic<br>
+// causes these to be laid out <br>
diff erently in the lambda.<br>
+// CHECK: define internal void<br>
+// CHECK: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 0<br>
+// CHECK: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 1<br>
+// CHECK: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 2<br>
+// CHECK: getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 3<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>