llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> See the comment I added for why this is weird. We might want to have a different mechanism for this in the future. --- Full diff: https://github.com/llvm/llvm-project/pull/107576.diff 4 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+10-3) - (modified) clang/lib/AST/ByteCode/Context.cpp (+10-6) - (modified) clang/lib/AST/ByteCode/InterpStack.cpp (+25) - (modified) clang/lib/AST/ByteCode/InterpStack.h (+1) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index a831f196abdcb5..2e9807b2d8426b 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -5507,11 +5507,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { if (isa<DecompositionDecl>(VD)) return revisit(VD); - // Visit local const variables like normal. - if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() || - VD->isStaticDataMember()) && + if ((VD->hasGlobalStorage() || VD->isStaticDataMember()) && typeShouldBeVisited(VD->getType())) return revisit(VD); + + // FIXME: The evaluateValue() check here is a little ridiculous, since + // it will ultimately call into Context::evaluateAsInitializer(). In + // other words, we're evaluating the initializer, just to know if we can + // evaluate the initializer. + if (VD->isLocalVarDecl() && typeShouldBeVisited(VD->getType()) && + VD->getInit() && !VD->getInit()->isValueDependent() && + VD->evaluateValue()) + return revisit(VD); } } else { if (const auto *VD = dyn_cast<VarDecl>(D); diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp index e682d87b703ad7..594c74cd3796d6 100644 --- a/clang/lib/AST/ByteCode/Context.cpp +++ b/clang/lib/AST/ByteCode/Context.cpp @@ -44,13 +44,14 @@ bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) { bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) { ++EvalID; bool Recursing = !Stk.empty(); + size_t StackSizeBefore = Stk.size(); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/E->isGLValue()); if (Res.isInvalid()) { C.cleanup(); - Stk.clear(); + Stk.clearTo(StackSizeBefore); return false; } @@ -60,7 +61,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) { #ifndef NDEBUG // Make sure we don't rely on some value being still alive in // InterpStack memory. - Stk.clear(); + Stk.clearTo(StackSizeBefore); #endif } @@ -72,12 +73,13 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) { bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) { ++EvalID; bool Recursing = !Stk.empty(); + size_t StackSizeBefore = Stk.size(); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); auto Res = C.interpretExpr(E); if (Res.isInvalid()) { C.cleanup(); - Stk.clear(); + Stk.clearTo(StackSizeBefore); return false; } @@ -87,7 +89,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) { #ifndef NDEBUG // Make sure we don't rely on some value being still alive in // InterpStack memory. - Stk.clear(); + Stk.clearTo(StackSizeBefore); #endif } @@ -99,6 +101,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD, APValue &Result) { ++EvalID; bool Recursing = !Stk.empty(); + size_t StackSizeBefore = Stk.size(); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); bool CheckGlobalInitialized = @@ -107,7 +110,8 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD, auto Res = C.interpretDecl(VD, CheckGlobalInitialized); if (Res.isInvalid()) { C.cleanup(); - Stk.clear(); + Stk.clearTo(StackSizeBefore); + return false; } @@ -117,7 +121,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD, #ifndef NDEBUG // Make sure we don't rely on some value being still alive in // InterpStack memory. - Stk.clear(); + Stk.clearTo(StackSizeBefore); #endif } diff --git a/clang/lib/AST/ByteCode/InterpStack.cpp b/clang/lib/AST/ByteCode/InterpStack.cpp index b8cdaeee72166c..ae3721e983741d 100644 --- a/clang/lib/AST/ByteCode/InterpStack.cpp +++ b/clang/lib/AST/ByteCode/InterpStack.cpp @@ -32,6 +32,16 @@ void InterpStack::clear() { #endif } +void InterpStack::clearTo(size_t NewSize) { + assert(NewSize <= size()); + size_t ToShrink = size() - NewSize; + if (ToShrink == 0) + return; + + shrink(ToShrink); + assert(size() == NewSize); +} + void *InterpStack::grow(size_t Size) { assert(Size < ChunkSize - sizeof(StackChunk) && "Object too large"); @@ -81,6 +91,21 @@ void InterpStack::shrink(size_t Size) { Chunk->End -= Size; StackSize -= Size; + +#ifndef NDEBUG + size_t TypesSize = 0; + for (PrimType T : ItemTypes) + TYPE_SWITCH(T, { TypesSize += aligned_size<T>(); }); + + size_t StackSize = size(); + while (TypesSize > StackSize) { + TYPE_SWITCH(ItemTypes.back(), { + TypesSize -= aligned_size<T>(); + ItemTypes.pop_back(); + }); + } + assert(TypesSize == StackSize); +#endif } void InterpStack::dump() const { diff --git a/clang/lib/AST/ByteCode/InterpStack.h b/clang/lib/AST/ByteCode/InterpStack.h index 153d17f0d70e1b..43988bb680d1c6 100644 --- a/clang/lib/AST/ByteCode/InterpStack.h +++ b/clang/lib/AST/ByteCode/InterpStack.h @@ -86,6 +86,7 @@ class InterpStack final { /// Clears the stack without calling any destructors. void clear(); + void clearTo(size_t NewSize); /// Returns whether the stack is empty. bool empty() const { return StackSize == 0; } `````````` </details> https://github.com/llvm/llvm-project/pull/107576 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
