Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 61 additions & 28 deletions src/passes/MemoryPacking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,34 @@ void MemoryPacking::run(Module* module) {
}
}

// Whether the byte range [offset, offset + size) provably fits in the
// declared minimum size of the memory, so that writing the segment cannot
// trap.
static bool
provablyInBounds(const Memory& memory, uint64_t offset, uint64_t size) {
uint64_t end;
if (std::ckd_add(&end, offset, size)) {
// The mathematical end is 2^64 + |end| (the wrapped value). That is in
// bounds only when it is exactly 2^64, that is, |end| is 0, and the
// memory's declared minimum size is the maximal 2^64 bytes. (With
// one-byte pages the largest declarable minimum is 2^64 - 1 bytes, so a
// segment ending at 2^64 is never in bounds there.)
if (end != 0 || memory.pageSizeLog2 == 0) {
return false;
}
return memory.initial == (uint64_t(1) << (64 - memory.pageSizeLog2));
}
Comment on lines +204 to +214

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this will return false when the memory size == offset + size == 2^64, when it could return true. Would that be too complicated to fix?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that corner was conservatively rejected: a maximal memory64 declares exactly 2^64 bytes, so a segment ending exactly at the memory end has a byte end that does not fit in uint64 and tripped the ckd_add bail. Not complicated to fix: on overflow, the mathematical end is 2^64 plus the wrapped value, which is in bounds only when the wrapped value is 0 and the declared minimum is the maximal size. (With one byte pages the largest declarable minimum is 2^64 - 1, so that case stays false, which also avoids a shift by 64.) Pushed, with a test where a segment at (i64.const -1) of size 1 no longer blocks the overlap optimization on a maximal memory64.

// Compare page counts rather than byte counts: the byte size of a maximal
// memory64 (2^48 pages) does not fit in 64 bits. Compute the number of
// pages needed to hold |end| bytes, rounding up as a partial page needs a
// whole page.
uint64_t endPages = end >> memory.pageSizeLog2;
if (end % memory.pageSize() != 0) {
endPages++;
}
return endPages <= memory.initial;
}

bool MemoryPacking::canOptimize(
std::vector<std::unique_ptr<Memory>>& memories,
std::vector<std::unique_ptr<DataSegment>>& dataSegments) {
Expand All @@ -215,7 +243,6 @@ bool MemoryPacking::canOptimize(
return true;
}
// Check if it is ok for us to optimize.
Address maxAddress = 0;
for (auto& segment : dataSegments) {
if (segment->isActive()) {
auto* c = segment->offset->dynCast<Const>();
Expand All @@ -241,9 +268,6 @@ bool MemoryPacking::canOptimize(
if (!c) {
return false;
}
// Note the maximum address so far.
maxAddress = std::max(
maxAddress, Address(c->value.getUnsigned() + segment->data.size()));
}
}
// All active segments have constant offsets, known at this time, so we may be
Expand All @@ -257,26 +281,39 @@ bool MemoryPacking::canOptimize(
DisjointSpans::Span span{start, start + segment->data.size()};
if (space.addAndCheckOverlap(span)) {
// Some segments overlap, that is, a later segment tramples the data of
// an earlier one. If the memory is imported then we cannot optimize
// here: if a later segment is out of bounds then instantiation traps
// partway, leaving the data written so far visible in the imported
// memory (which outlives the failed instantiation), so even trampled
// data matters.
// TODO: We could optimize anyway if we can check that all the segments
// after the trampled segment, up to and including the trampling
// segment, will be in-bounds for the imported memory, as then no
// trap can occur between the trampled write and the trampling
// one.
// an earlier one. If the memory is imported then a trap on a later
// out-of-bounds segment leaves the data written so far visible in the
// imported memory (which outlives the failed instantiation), so even
// trampled data matters. We can only optimize if we prove no active
// segment can trap, which is the case when each is in bounds of the
// memory's declared minimum size: then instantiation always applies
// every segment, and only the final memory contents are observable,
// exactly as for a memory defined in the module.
// TODO: This could be finer-grained: it is enough for the segments
// after a trampled segment, up to and including its trampling
// segment, to be provably in-bounds, as then no trap can occur
// between the trampled write and the trampling one.
if (memory->imported()) {
std::cerr << "warning: active memory segments have overlap, which "
<< "prevents some optimizations.\n";
return false;
for (auto& seg : dataSegments) {
if (seg->isActive() &&
!provablyInBounds(
*memory,
seg->offset->cast<Const>()->value.getUnsigned(),
seg->data.size())) {
std::cerr
<< "warning: active memory segments overlap, and some may "
<< "be out of bounds of the imported memory's declared "
<< "minimum size, which prevents some optimizations.\n";
return false;
}
}
}
// The memory is defined in this module, so partially-applied segments
// can never be observed: either instantiation completes and all the
// segments are applied in order, or it traps and the memory is never
// exposed. We can therefore zero out the trampled data, which the
// normal optimization of zeros will then remove.
// Zero out the trampled data, which the normal optimization of zeros
// will then remove. For a memory defined in the module
// partially-applied segments can never be observed (either
// instantiation completes and all the segments are applied in order,
// or it traps and the memory is never exposed), and for an imported
// memory we just proved that no trap is possible.
zeroOutTrampledData(dataSegments);
break;
}
Expand Down Expand Up @@ -388,12 +425,8 @@ void MemoryPacking::calculateRanges(Module* module,
// Check if we can rule out a trap by it being in bounds.
if (auto* c = segment->offset->dynCast<Const>()) {
auto* memory = module->getMemory(segment->memory);
auto memorySize = memory->initial << memory->pageSizeLog2;
Index start = c->value.getUnsigned();
Index size = segment->data.size();
Index end;
if (!std::ckd_add(&end, start, size) && end <= memorySize) {
// This is in bounds.
if (provablyInBounds(
*memory, c->value.getUnsigned(), segment->data.size())) {
preserveTrap = false;
}
}
Expand Down
101 changes: 98 additions & 3 deletions test/lit/passes/memory-packing_zero-filled-memory.wast
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,110 @@

;; CHECK: (data $0 (i32.const 1024) "x")
(module
;; but we cannot optimize trampling on an imported memory: if a later segment
;; were to trap during instantiation, the data written before it remains
;; visible in the imported memory, so even the trampled "x" must be kept
;; we can optimize trampling on an imported memory when every active segment
;; is provably in bounds of the declared minimum size: then no segment can
;; trap during instantiation, so only the final memory contents are
;; observable, and the trampled "x" is zeroed out and removed along with the
;; zero that tramples it.
;; CHECK: (import "env" "memory" (memory $0 1 1))
(import "env" "memory" (memory $0 1 1))

(data (i32.const 1024) "x")
(data (i32.const 1024) "\00")
)

(module
;; the same, but with a nonzero trampling byte: the trampled "a" is removed
;; while the rest of the segment and the trampler are kept.
;; CHECK: (import "env" "memory" (memory $0 1 1))
(import "env" "memory" (memory $0 1 1))

(data (i32.const 1024) "ab")
(data (i32.const 1024) "Z")
)

;; CHECK: (data $0 (i32.const 1025) "b")

;; CHECK: (data $1 (i32.const 1024) "Z")
(module
;; but we cannot optimize when the trampling segment may be out of bounds
;; (here it ends one byte past the declared minimum size of one page): if it
;; traps during instantiation, the data written before it remains visible in
;; the imported memory, so even the trampled "x" must be kept
;; CHECK: (import "env" "memory" (memory $0 1 1))
(import "env" "memory" (memory $0 1 1))

(data (i32.const 65535) "x")
(data (i32.const 65535) "\00\00")
)

;; CHECK: (data $0 (i32.const 65535) "x")

;; CHECK: (data $1 (i32.const 65535) "\00\00")
(module
;; here the overlapping segments are both in bounds, but a possibly
;; out-of-bounds segment elsewhere in the module still prevents the
;; optimization: if it traps, the trampled "x" remains visible. (This is more
;; conservative than necessary, as that segment is applied only after the
;; trampling one; see the TODO in canOptimize.)
;; CHECK: (import "env" "memory" (memory $0 1 1))
(import "env" "memory" (memory $0 1 1))

(data (i32.const 1024) "x")
(data (i32.const 1024) "\00")
(data (i32.const 65535) "yy")
)
;; CHECK: (data $0 (i32.const 1024) "x")

;; CHECK: (data $1 (i32.const 1024) "\00")

;; CHECK: (data $2 (i32.const 65535) "yy")
(module
;; a segment with a non-constant offset (here an imported global, as in
;; dynamic linking) prevents any reasoning about overlap or bounds when
;; there are multiple segments: we cannot tell what it tramples or whether
;; it traps, so nothing is optimized and the zero is kept.
;; CHECK: (import "env" "memory" (memory $0 1 1))
(import "env" "memory" (memory $0 1 1))

;; CHECK: (import "env" "offset" (global $offset i32))
(import "env" "offset" (global $offset i32))

(data (i32.const 1024) "x")
(data (global.get $offset) "\00")
)
;; CHECK: (data $0 (i32.const 1024) "x")

;; CHECK: (data $1 (global.get $offset) "\00")
(module
;; a memory64 memory with the maximal declared minimum size (2^48 pages):
;; the size in bytes does not fit in 64 bits, but the in-bounds check
;; compares page counts, so the overlapping segments are still optimized.
;; CHECK: (import "env" "memory" (memory $0 i64 281474976710656 281474976710656))
(import "env" "memory" (memory $0 i64 281474976710656 281474976710656))

(data (i64.const 1024) "x")
(data (i64.const 1024) "\00")
)
(module
;; a memory64 segment whose offset does not fit in 32 bits and is out of
;; bounds of the one-page memory: it traps during instantiation, so its
;; zeros must not be removed. (This used to truncate the offset to 32 bits,
;; conclude the segment was in bounds, and remove the trap.)
;; CHECK: (import "env" "memory" (memory $0 i64 1 1))
(import "env" "memory" (memory $0 i64 1 1))

(data (i64.const 4294968320) "\00\00")
)
;; CHECK: (data $0 (i64.const 4294968321) "\00")
(module
;; a segment ending exactly at 2^64 bytes, the size of this maximal memory64:
;; the byte end does not fit in 64 bits, but it is still provably in bounds,
;; so the overlap optimization applies and all the segments are removed.
;; CHECK: (import "env" "memory" (memory $0 i64 281474976710656 281474976710656))
(import "env" "memory" (memory $0 i64 281474976710656 281474976710656))

(data (i64.const 1024) "x")
(data (i64.const 1024) "\00")
(data (i64.const -1) "\00")
)
Loading