Handle constant offsets larger than 4G in memory64-lowering#8316
Handle constant offsets larger than 4G in memory64-lowering#8316
Conversation
If memory instructions have constant offsets greater than the maxiumum size of a 32-bit memory, they become invalid when the memory becomes a 32-bit memory. In this case the memory operation can never succeed, and if the operation was legtimate in the first place (rather than e.g. being the result of an optimization that took advantage of UB in C), then the program cannot be correctly lowered. So in anny case it is better to trap if the instruction is actually executed.
src/passes/Memory64Lowering.cpp
Outdated
| if (curr->offset < k32GLimit) { | ||
| return wrapAddress64(curr->ptr, curr->memory); | ||
| } | ||
| replaceCurrent(Builder(*getModule()).makeUnreachable()); |
There was a problem hiding this comment.
This is not quite right as it replaces the entire tree, including the children. A child may branch away, so the program might have executed validly before this.
See ChildLocalizer::getChildrenReplacement() which returns the children of an expression. That returns a block which you can append an unreachable to.
There was a problem hiding this comment.
Thanks, I think I have it. Do you think we need a more complex test that would cover this (beyond the current test that just shows an empty block?) Or maybe that would just be testing ChildLocalizer itself and should be done elsewhere...
| ;; CHECK-NEXT: ) | ||
| (func $test_simd_load_large_offset (param $ptr i64) | ||
| (drop (v128.load offset=4294967296 (local.get $ptr))) | ||
| ) |
There was a problem hiding this comment.
When I turn on pass debug I'm getting a validation error:
[wasm-validator error in function test_simd_load_large_offset] stale type found in test_simd_load_large_offset on 0x5647b782bca8
(marked as none, should be unreachable)
, on
(drop
(block
(unreachable)
)
)
I guess I need to propagate the unreachable type from the block up to the drop? Is there a utility for that?
There was a problem hiding this comment.
Oh, yes, if you add an unreachable that is a type change. You can note that and then run ReFinalize().walkFunctionInModule(func, module); on the entire function. See e.g. the Heap2Local pass.
There was a problem hiding this comment.
Thanks, does this look right?
2 participants
If memory instructions have constant offsets greater than the maxiumum size of
a 32-bit memory, they become invalid when the memory becomes a 32-bit memory.
In this case the memory operation can never succeed, and if the operation
was legtimate in the first place (rather than e.g. being the result of
an optimization that took advantage of UB in C), then the program cannot be
correctly lowered. So in any case it is better to trap if the instruction
is actually executed.
So this change replaces memory operations with large constant offsets with an
unreachable, and hoists any child operations into a block that runs before it.