diff --git a/src/passes/MemoryPacking.cpp b/src/passes/MemoryPacking.cpp index c1c0297f6d0..ec74bf4f529 100644 --- a/src/passes/MemoryPacking.cpp +++ b/src/passes/MemoryPacking.cpp @@ -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)); + } + // 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>& memories, std::vector>& dataSegments) { @@ -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(); @@ -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 @@ -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()->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; } @@ -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()) { 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; } } diff --git a/test/lit/passes/memory-packing_zero-filled-memory.wast b/test/lit/passes/memory-packing_zero-filled-memory.wast index c4b9e48f602..9500848309f 100644 --- a/test/lit/passes/memory-packing_zero-filled-memory.wast +++ b/test/lit/passes/memory-packing_zero-filled-memory.wast @@ -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") +)