[clang] 1eddce4 - Fix non-determinism issue with implicit lambda captures.

[Richard Smith via cfe-commits]

On Fri, 12 Jun 2020 at 09:17, Erich Keane via cfe-commits <
cfe-commits at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200612/dabce716/attachment-0001.html>