rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Looks like a good first step, thanks!
================
Comment at: clang/include/clang/AST/ASTContext.h:2821-2823
+ /// Adds an APValue that will be destructed during the destruction of the
+ /// ASTContext.
+ void AddAPValueCleanup(APValue *Ptr) const { APValueCleanups.push_back(Ptr);
}
----------------
We'll need cleanups for `APSInt`s too, in the case where they don't fit into
their inline storage. Maybe instead of storing a trailing `APSInt` we could
store a trailing `uint64_t` (and put the bit-width and `Signed` flag into the
`ConstantExprBits`), and use the `APValue` representation for `APSInt`s that
don't fit in 64 bits? (That'd avoid needing a separate list of cleanups and
would also make `ConstantExpr` 16 bytes smaller in the common case of storing
an int that fits in 64 bits.)
As an alternative, we could tail-allocate a sequence of `uint64_t`s for an
`APSInt` that's larger than 64 bits, but that would force a memory allocation
and a copy on every access to the value, so I think it's not worthwhile.
================
Comment at: clang/include/clang/Serialization/ASTWriter.h:866
+ /// Emit an integral value.
+ void AddAPValue(const APValue &Value);
----------------
Comment needs to be updated.
================
Comment at: clang/lib/AST/Expr.cpp:241
+ switch (Kind) {
+ case APValue::Uninitialized:
+ return ConstantExpr::RSK_None;
----------------
After recent changes, this has been split into `APValue::None` and
`APValue::Indeterminate`.
================
Comment at: clang/lib/Serialization/ASTReader.cpp:9108-9110
+ uint64_t Tmp = Record[Idx++];
+ unsigned Width = Tmp & 0xffffffff;
+ unsigned Scale = Tmp >> static_cast<uint64_t>(32);
----------------
Record values in AST files are typically stored with VBR6 encoding, as
described here: https://llvm.org/docs/BitCodeFormat.html#variable-width-value
In particular, that encoding means it's usually more space-efficient to store a
pair of two 32-bit integers as two separate fields rather than packing the bits
of the integers into one field like this. (If you bit-pack like this, then you
always use at least 7 VBR6 fields = 42 bits to store these two values, whereas
if you treat them as separate fields and both are small, they can fit into 2
VBR6 fields = 12 bits.)
So please store `Width` and `Scale` as two separate vector entries :)
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62399/new/
https://reviews.llvm.org/D62399
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
