[clang][bytecode] Speed up EvaluationResult::CheckArrayInitialized()

[tbaederr]

For large primitive arrays, avoid creating a new Pointer for every element (via Pointer::isElementInitialized()) or avoid iterating over the array altogether (via Pointer::allElementsInitialized()).

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

For large primitive arrays, avoid creating a new Pointer for every element (via Pointer::isElementInitialized()) or avoid iterating over the array altogether (via Pointer::allElementsInitialized()).

---

Full diff: https://github.com/llvm/llvm-project/pull/155756.diff

3 Files Affected:

• (modified) clang/lib/AST/ByteCode/EvaluationResult.cpp (+11-1)
• (modified) clang/lib/AST/ByteCode/Pointer.cpp (+49-16)
• (modified) clang/lib/AST/ByteCode/Pointer.h (+5-2)

diff --git a/clang/lib/AST/ByteCode/EvaluationResult.cpp b/clang/lib/AST/ByteCode/EvaluationResult.cpp index 5110e243d167c..2c14ac0b88845 100644 --- a/clang/lib/AST/ByteCode/EvaluationResult.cpp +++ b/clang/lib/AST/ByteCode/EvaluationResult.cpp @@ -82,8 +82,18 @@ static bool CheckArrayInitialized(InterpState &S, SourceLocation Loc, Result &= CheckArrayInitialized(S, Loc, ElemPtr, ElemCAT); } } else { + // Primitive arrays. + if (S.getContext().canClassify(ElemType)) { + if (BasePtr.allElementsInitialized()) { + return true; + } else { + DiagnoseUninitializedSubobject(S, Loc, BasePtr.getField()); + return false; + } + } + for (size_t I = 0; I != NumElems; ++I) { - if (!BasePtr.atIndex(I).isInitialized()) { + if (!BasePtr.isElementInitialized(I)) { DiagnoseUninitializedSubobject(S, Loc, BasePtr.getField()); Result = false; } diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp index 9fc5b5183f5f6..973bc7cbad62a 100644 --- a/clang/lib/AST/ByteCode/Pointer.cpp +++ b/clang/lib/AST/ByteCode/Pointer.cpp @@ -442,26 +442,42 @@ bool Pointer::isInitialized() const { assert(BS.Pointee && "Cannot check if null pointer was initialized"); const Descriptor *Desc = getFieldDesc(); assert(Desc); - if (Desc->isPrimitiveArray()) { - if (isStatic() && BS.Base == 0) - return true; + if (Desc->isPrimitiveArray()) + return isElementInitialized(getIndex()); - InitMapPtr &IM = getInitMap(); + if (asBlockPointer().Base == 0) + return true; + // Field has its bit in an inline descriptor. + return getInlineDesc()->IsInitialized; +} + +bool Pointer::isElementInitialized(unsigned Index) const { + if (!isBlockPointer()) + return true; + + const Descriptor *Desc = getFieldDesc(); + assert(Desc); + + if (isStatic() && BS.Base == 0) + return true; + + if (isRoot() && BS.Base == sizeof(GlobalInlineDescriptor)) { + const GlobalInlineDescriptor &GD = + *reinterpret_cast<const GlobalInlineDescriptor *>(block()->rawData()); + return GD.InitState == GlobalInitState::Initialized; + } + if (Desc->isPrimitiveArray()) { + InitMapPtr &IM = getInitMap(); if (!IM) return false; if (IM->first) return true; - return IM->second->isElementInitialized(getIndex()); + return IM->second->isElementInitialized(Index); } - - if (asBlockPointer().Base == 0) - return true; - - // Field has its bit in an inline descriptor. - return getInlineDesc()->IsInitialized; + return isInitialized(); } void Pointer::initialize() const { @@ -524,6 +540,23 @@ void Pointer::initializeAllElements() const { } } +bool Pointer::allElementsInitialized() const { + assert(getFieldDesc()->isPrimitiveArray()); + assert(isArrayRoot()); + + if (isStatic() && BS.Base == 0) + return true; + + if (isRoot() && BS.Base == sizeof(GlobalInlineDescriptor)) { + const GlobalInlineDescriptor &GD = + *reinterpret_cast<const GlobalInlineDescriptor *>(block()->rawData()); + return GD.InitState == GlobalInitState::Initialized; + } + + InitMapPtr &IM = getInitMap(); + return IM && IM->first; +} + void Pointer::activate() const { // Field has its bit in an inline descriptor. assert(BS.Base != 0 && "Only composite fields can be activated"); @@ -771,13 +804,13 @@ std::optional<APValue> Pointer::toRValue(const Context &Ctx, R = APValue(APValue::UninitArray{}, NumElems, NumElems); bool Ok = true; - for (unsigned I = 0; I < NumElems; ++I) { + OptPrimType ElemT = Ctx.classify(ElemTy); + for (unsigned I = 0; I != NumElems; ++I) { APValue &Slot = R.getArrayInitializedElt(I); - const Pointer &EP = Ptr.atIndex(I); - if (OptPrimType T = Ctx.classify(ElemTy)) { - TYPE_SWITCH(*T, Slot = EP.deref<T>().toAPValue(ASTCtx)); + if (ElemT) { + TYPE_SWITCH(*ElemT, Slot = Ptr.elem<T>(I).toAPValue(ASTCtx)); } else { - Ok &= Composite(ElemTy, EP.narrow(), Slot); + Ok &= Composite(ElemTy, Ptr.atIndex(I).narrow(), Slot); } } return Ok; diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h index b2e9786b8ac7a..49d701c3e27b6 100644 --- a/clang/lib/AST/ByteCode/Pointer.h +++ b/clang/lib/AST/ByteCode/Pointer.h @@ -528,8 +528,6 @@ class Pointer { assert(isBlockPointer()); return BS.Pointee->isWeak(); } - /// Checks if an object was initialized. - bool isInitialized() const; /// Checks if the object is active. bool isActive() const { if (!isBlockPointer()) @@ -707,6 +705,11 @@ class Pointer { /// used in situations where we *know* we have initialized *all* elements /// of a primtive array. void initializeAllElements() const; + /// Checks if an object was initialized. + bool isInitialized() const; + /// Like isInitialized(), but for primitive arrays. + bool isElementInitialized(unsigned Index) const; + bool allElementsInitialized() const; /// Activats a field. void activate() const; /// Deactivates an entire strurcutre. 

[shafik]

You use this check several times in Pointer.cpp

isRoot() && BS.Base == sizeof(GlobalInlineDescriptor)

and it is not obvious what it means, seems like it should have a name via a member function to avoid repeating ourselves.

[shafik]

This is always the problem w/ std::pair what does first and second mean. I looked up the declaration of InitMapPtr but there is no helpful comment there telling me what the first bool means. I would either suggest not using std::pair or commenting at the declaration explaining the meaning.