steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun, Szelethus, vsavchenko, ASDenysPetrov.
Herald added subscribers: cfe-commits, martong, Charusso, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.
Herald added a project: clang.
steakhal requested review of this revision.
By the time working on the gdb pretty-printers for the Analyzer classes,
I observed that the `MemRegion` class has an inefficient memory layout.
Each and every object which's has a `MemRegion` as a subobject would benefit
from this change.
We could spare 8 bytes for each case.
As a downside, I had to substitute the `llvm::Optional`, since that alone
consumes 16 bytes (even if the wrapped object would consume 8 bytes :| ).
Before the patch: `sizeof(MemRegion) == 48`
(gdb) ptype /o class MemRegion
/* offset | size */ type = class clang::ento::MemRegion : public
llvm::FoldingSetBase::Node {
private:
/* 16 | 4 */ const clang::ento::MemRegion::Kind kind;
/* XXX 4-byte hole */
/* 24 | 24 */ class llvm::Optional<clang::ento::RegionOffset>
[with T = clang::ento::RegionOffset] {
private:
/* 24 | 24 */ class
llvm::optional_detail::OptionalStorage<T, true> [with T = T] {
private:
/* 24 | 16 */ union {
/* 1 */ char empty;
/* 16 */ T value;
/* total size (bytes): 16 */
};
/* 40 | 1 */ bool hasVal;
/* total size (bytes): 24 */
} Storage;
/* total size (bytes): 24 */
} cachedOffset;
/* total size (bytes): 48 */
}
After the patch: `sizeof(MemRegion) == 40`
(gdb) ptype /o MemRegion
/* offset | size */ type = class clang::ento::MemRegion : public
llvm::FoldingSetBase::Node {
private:
/* 16 | 16 */ class clang::ento::RegionOffset {
private:
/* 16 | 8 */ const clang::ento::MemRegion *R;
/* 24 | 8 */ int64_t Offset;
public:
static const int64_t Symbolic;
/* total size (bytes): 16 */
} cachedOffset;
/* 32 | 4 */ const clang::ento::MemRegion::Kind kind;
/* 36 | 1 */ bool hasCachedOffset;
/* total size (bytes): 40 */
}
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86295
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1579,9 +1579,11 @@
}
RegionOffset MemRegion::getAsOffset() const {
- if (!cachedOffset)
+ if (!hasCachedOffset) {
cachedOffset = calculateOffset(this);
- return *cachedOffset;
+ hasCachedOffset = true;
+ }
+ return cachedOffset;
}
//===----------------------------------------------------------------------===//
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -100,8 +100,9 @@
};
private:
+ mutable RegionOffset cachedOffset;
const Kind kind;
- mutable Optional<RegionOffset> cachedOffset;
+ mutable bool hasCachedOffset = false;
protected:
MemRegion(Kind k) : kind(k) {}
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1579,9 +1579,11 @@
}
RegionOffset MemRegion::getAsOffset() const {
- if (!cachedOffset)
+ if (!hasCachedOffset) {
cachedOffset = calculateOffset(this);
- return *cachedOffset;
+ hasCachedOffset = true;
+ }
+ return cachedOffset;
}
//===----------------------------------------------------------------------===//
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -100,8 +100,9 @@
};
private:
+ mutable RegionOffset cachedOffset;
const Kind kind;
- mutable Optional<RegionOffset> cachedOffset;
+ mutable bool hasCachedOffset = false;
protected:
MemRegion(Kind k) : kind(k) {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
