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