Reuse the patterns from WebAssemblyInstrMemory.td.
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 22079 Build 22079: arc lint + arc unit
Event Timeline
lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h | ||
---|---|---|
298 | I'm not actually sure these are in the right place. It looks like this switch returns the log of the default alignment for each type? The spec proposal says, "the memory access size is 16 bytes which is also the natural alignment." Am I reading that correctly to mean that these cases should return 4, not 3? |
lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h | ||
---|---|---|
298 | Yes, that's correct; I'm pretty sure the intention is that the alignment is 16 (4 here). This is a requirement/optimization for some (sub)architectures such as SSE2 (and maybe ARM? not sure), as well as some native C ABIs. |
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td | ||
---|---|---|
264 | Do we have tests for these load/store offset patterns? Tests for scalars are here, and I think we need similar variation of tests for SIMDs too. We probably wouldn't need all of offset variation tests for all existing vector types though. Maybe offset tests for one SIMD type would be sufficient. |
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td | ||
---|---|---|
264 | I implemented the tests for all types for now, since the marginal cost to do so was small. If we value having a smaller test file more than having all the types covered, I can remove some of the tests. |
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td | ||
---|---|---|
264 | Yeah it looks like you wrote it once and auto-generated for all other types so, more tests wouldn't hurt. :) | |
test/CodeGen/WebAssembly/simd-memory.ll | ||
1 | How about changing filename to offset-simd.ll or something, because it looks like other offset-testing tests follow the same convention. |
test/CodeGen/WebAssembly/offset-simd.td | ||
---|---|---|
1 ↗ | (On Diff #163403) | Oh not .td but .ll |
test/CodeGen/WebAssembly/offset-simd.td | ||
---|---|---|
1 ↗ | (On Diff #163403) | Oops, fixed. |
I'm not actually sure these are in the right place. It looks like this switch returns the log of the default alignment for each type? The spec proposal says, "the memory access size is 16 bytes which is also the natural alignment." Am I reading that correctly to mean that these cases should return 4, not 3?