tuples: Treat Type::timestamp as an array #1658
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we were seeing issues like:
on certain LLVM versions. The issue most likely has to do with padding
and whether LLVM considers the 16 byte type as a single 128 bit integer
or two 64 bit integers. If considered as a single 128 bit integer, then
the padded struct size is indeed 32 bytes. If as two 64 bit integers,
then 24 bytes is correct.
Instead of trying to fight with LLVM on this, mark Type::timestamp as an
array so that codegen will tell LLVM that Type::timestamp types are an
array of 16 8-bit integers. This effectively packs the type. We know
this is safe to do b/c Type::timestamp types are accessed from userspace
as two 64-bit integers anyways (see
AsyncEvent::Strftime
) which isnaturally packed on both 32-bit and 64-bit machines.
Note that this is also how Type::Usym is implemented.
Checklist
docs/reference_guide.md
CHANGELOG.md