r244989 - Avoid iteration invalidation issues around MaterializedTemporaryExpr
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 25 08:35:25 PST 2016
Seems like this test got flagged as 'slow' by Google's internal
infrastructure - and that makes me wonder about whether it's appropriate to
have in the lit test suite - we really want to keep these tests as fast as
possible.
I think we're generally OK committing iterator invalidation fixes without
tests in the past, and I'd be inclined to suggest removing this test. It's
nice to have the demonstration to justify the commit, but I'm not sure
it'll pull its weight as part of the regression suite.
(unicorn: what I'd love to see is a checked mode for our containers, so
that we could reduce invalidation test cases down to something entirely
reasonable and that would fail fast without having to concoct a rehashing
event to occur at a particular point)
On Thu, Aug 13, 2015 at 4:50 PM, David Majnemer via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: majnemer
> Date: Thu Aug 13 18:50:15 2015
> New Revision: 244989
>
> URL: http://llvm.org/viewvc/llvm-project?rev=244989&view=rev
> Log:
> Avoid iteration invalidation issues around MaterializedTemporaryExpr
>
> We risk iterator invalidation issues if we use a DenseMap to hold the
> backing storage for an APValue. Instead, BumpPtrAllocate them and
> use APValue * as our DenseMap value.
>
> Also, don't assume that MaterializedGlobalTemporaryMap won't regrow
> between when we initially perform a lookup and later on when we actually
> try to insert into it.
>
> This fixes PR24289.
>
> Differential Revision: http://reviews.llvm.org/D11629
>
> Added:
> cfe/trunk/test/CodeGenCXX/PR24289.cpp
> Modified:
> cfe/trunk/include/clang/AST/ASTContext.h
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=244989&r1=244988&r2=244989&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Thu Aug 13 18:50:15 2015
> @@ -176,8 +176,9 @@ class ASTContext : public RefCountedBase
> ClassScopeSpecializationPattern;
>
> /// \brief Mapping from materialized temporaries with static storage
> duration
> - /// that appear in constant initializers to their evaluated values.
> - llvm::DenseMap<const MaterializeTemporaryExpr*, APValue>
> + /// that appear in constant initializers to their evaluated values.
> These are
> + /// allocated in a std::map because their address must be stable.
> + llvm::DenseMap<const MaterializeTemporaryExpr *, APValue *>
> MaterializedTemporaryValues;
>
> /// \brief Representation of a "canonical" template template parameter
> that
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=244989&r1=244988&r2=244989&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Aug 13 18:50:15 2015
> @@ -8553,12 +8553,14 @@ ASTContext::getMaterializedTemporaryValu
> bool MayCreate) {
> assert(E && E->getStorageDuration() == SD_Static &&
> "don't need to cache the computed value for this temporary");
> - if (MayCreate)
> - return &MaterializedTemporaryValues[E];
> + if (MayCreate) {
> + APValue *&MTVI = MaterializedTemporaryValues[E];
> + if (!MTVI)
> + MTVI = new (*this) APValue;
> + return MTVI;
> + }
>
> - llvm::DenseMap<const MaterializeTemporaryExpr *, APValue>::iterator I =
> - MaterializedTemporaryValues.find(E);
> - return I == MaterializedTemporaryValues.end() ? nullptr : &I->second;
> + return MaterializedTemporaryValues.lookup(E);
> }
>
> bool ASTContext::AtomicUsesUnsupportedLibcall(const AtomicExpr *E) const {
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=244989&r1=244988&r2=244989&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Aug 13 18:50:15 2015
> @@ -3029,8 +3029,7 @@ llvm::Constant *CodeGenModule::GetAddrOf
> if (Init == E->GetTemporaryExpr())
> MaterializedType = E->getType();
>
> - llvm::Constant *&Slot = MaterializedGlobalTemporaryMap[E];
> - if (Slot)
> + if (llvm::Constant *Slot = MaterializedGlobalTemporaryMap[E])
> return Slot;
>
> // FIXME: If an externally-visible declaration extends multiple
> temporaries,
> @@ -3101,7 +3100,7 @@ llvm::Constant *CodeGenModule::GetAddrOf
> GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
> if (VD->getTLSKind())
> setTLSMode(GV, *VD);
> - Slot = GV;
> + MaterializedGlobalTemporaryMap[E] = GV;
> return GV;
> }
>
>
> Added: cfe/trunk/test/CodeGenCXX/PR24289.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/PR24289.cpp?rev=244989&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/PR24289.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/PR24289.cpp Thu Aug 13 18:50:15 2015
> @@ -0,0 +1,82 @@
> +// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-pc-linux-gnu
> -std=c++11 | FileCheck %s
> +
> +namespace std {
> +template <class T>
> +struct initializer_list {
> + const T *Begin;
> + __SIZE_TYPE__ Size;
> +
> + constexpr initializer_list(const T *B, __SIZE_TYPE__ S)
> + : Begin(B), Size(S) {}
> +};
> +}
> +
> +void f() {
> + static std::initializer_list<std::initializer_list<int>> a{
> + {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0},
> {0}};
> + static std::initializer_list<std::initializer_list<int>> b{
> + {0}, {0}, {0}, {0}};
> + static std::initializer_list<std::initializer_list<int>> c{
> + {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}};
> + static std::initializer_list<std::initializer_list<int>> d{
> + {0}, {0}, {0}, {0}, {0}};
> + static std::initializer_list<std::initializer_list<int>> e{
> + {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}, {0}};
> +}
> +
> +// CHECK-DAG: @_ZZ1fvE1a = internal global %{{.*}} { %{{.*}}*
> getelementptr inbounds ([14 x %{{.*}}], [14 x %{{.*}}]
> +// CHECK-DAG: * @_ZGRZ1fvE1a_, i32 0, i32 0), i64 14 }
> +// CHECK-DAG: @_ZGRZ1fvE1a0_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a1_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a2_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a3_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a4_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a5_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a6_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a7_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a8_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a9_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1aA_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1aB_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1aC_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1aD_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1a_ = internal constant [14 x %{{.*}}] [%{{.*}} {
> i32* getelementptr inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1a0_, i32 0,
> i32 0), i64 1 }, %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x
> i32]* @_ZGRZ1fvE1a1_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1a2_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1a3_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1a4_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1a5_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1a6_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1a7_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1a8_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelemen
> tptr inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1a9_, i32 0, i32 0), i64 1
> }, %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1aA_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1aB_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1aC_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1aD_, i32 0, i32 0), i64 1 }]
> +// CHECK-DAG: @_ZZ1fvE1b = internal global %{{.*}} { %{{.*}}*
> getelementptr inbounds ([4 x %{{.*}}], [4 x %{{.*}}]*
> +// CHECK-DAG: @_ZGRZ1fvE1b_, i32 0, i32 0), i64 4 }
> +// CHECK-DAG: @_ZGRZ1fvE1b0_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1b1_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1b2_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1b3_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1b_ = internal constant [4 x %{{.*}}] [%{{.*}} {
> i32* getelementptr inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1b0_, i32 0,
> i32 0), i64 1 }, %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x
> i32]* @_ZGRZ1fvE1b1_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1b2_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1b3_, i32 0, i32 0), i64 1 }]
> +// CHECK-DAG: @_ZZ1fvE1c = internal global %{{.*}} { %{{.*}}*
> getelementptr inbounds ([9 x %{{.*}}], [9 x %{{.*}}]*
> +// CHECK-DAG: @_ZGRZ1fvE1c_, i32 0, i32 0), i64 9 }
> +// CHECK-DAG: @_ZGRZ1fvE1c0_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c1_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c2_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c3_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c4_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c5_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c6_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c7_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c8_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1c_ = internal constant [9 x %{{.*}}] [%{{.*}} {
> i32* getelementptr inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1c0_, i32 0,
> i32 0), i64 1 }, %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x
> i32]* @_ZGRZ1fvE1c1_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1c2_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1c3_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1c4_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1c5_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1c6_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1c7_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1c8_, i32 0, i32 0), i64 1 }]
> +// CHECK-DAG: @_ZZ1fvE1d = internal global %{{.*}} { %{{.*}}*
> getelementptr inbounds ([5 x %{{.*}}], [5 x %{{.*}}]* @_ZGRZ1fvE1d_, i32 0,
> i32 0), i64 5 }
> +// CHECK-DAG: @_ZGRZ1fvE1d0_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1d1_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1d2_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1d3_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1d4_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1d_ = internal constant [5 x %{{.*}}] [%{{.*}} {
> i32* getelementptr inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1d0_, i32 0,
> i32 0), i64 1 }, %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x
> i32]* @_ZGRZ1fvE1d1_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1d2_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1d3_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1d4_, i32 0, i32 0), i64 1 }]
> +// CHECK-DAG: @_ZZ1fvE1e = internal global %{{.*}} { %{{.*}}*
> getelementptr inbounds ([11 x %{{.*}}], [11 x %{{.*}}]* @_ZGRZ1fvE1e_, i32
> 0, i32 0), i64 11 }
> +// CHECK-DAG: @_ZGRZ1fvE1e0_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e1_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e2_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e3_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e4_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e5_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e6_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e7_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e8_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e9_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1eA_ = internal constant [1 x i32] zeroinitializer
> +// CHECK-DAG: @_ZGRZ1fvE1e_ = internal constant [11 x %{{.*}}] [%{{.*}} {
> i32* getelementptr inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1e0_, i32 0,
> i32 0), i64 1 }, %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x
> i32]* @_ZGRZ1fvE1e1_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1e2_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1e3_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1e4_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1e5_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1e6_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1e7_, i32 0, i32 0), i64 1 }, %{{.*}} { i32* getelementptr
> inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1e8_, i32 0, i32 0), i64 1 },
> %{{.*}} { i32* getelemen
> tptr inbounds ([1 x i32], [1 x i32]* @_ZGRZ1fvE1e9_, i32 0, i32 0), i64 1
> }, %{{.*}} { i32* getelementptr inbounds ([1 x i32], [1 x i32]*
> @_ZGRZ1fvE1eA_, i32 0, i32 0), i64 1 }]
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://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/20160225/e0bcc422/attachment-0001.html>
More information about the cfe-commits
mailing list