WebAssembly loads and stores have an alignment attribute, so they support any alignment efficiently, provided that the alignment is correctly declared. This patch adds support for correctly declaring alignment values.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp | ||
---|---|---|
180 ↗ | (On Diff #45867) | Could you name the method differently do it reflects the power-of-two-byte unit? |
lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h | ||
50 ↗ | (On Diff #45867) | OPERAND_P2ALIGNMENT? |
lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp | ||
87 ↗ | (On Diff #45867) | Can you line-comment that this is p2align? I find the implicit contracts in MI to be hard to understand when reading the code years after it was written :-) |
lib/Target/WebAssembly/WebAssemblyISelLowering.cpp | ||
294 ↗ | (On Diff #45867) | Should this have: if (Fast) *Fast = false; To discourage that code generation? |
lib/Target/WebAssembly/WebAssemblyISelLowering.h | ||
61 ↗ | (On Diff #45867) | Can you drop the defaults? The override doesn't need them I think. |
lib/Target/WebAssembly/WebAssemblyPeephole.cpp | ||
74 ↗ | (On Diff #45867) | Could you add symbolic constants for the operand numbers? 3 -> 4 is pretty obscure :-) |
lib/Target/WebAssembly/WebAssemblySetAlignments.cpp | ||
11 ↗ | (On Diff #45867) | Sad that you need this pass at all. |
lib/Target/WebAssembly/WebAssemblyISelLowering.cpp | ||
---|---|---|
294 ↗ | (On Diff #45867) | Actually, I believe we want to encourage that code generation. On hardware that doesn't support fast unaligned accesses, engines should compile i64.store align=4 into two stores anyway, so it's better for the wasm to have that than two explicit stores. I added a comment for this. |
lgtm after typo fix. Very nice :)
llvm/trunk/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp | ||
---|---|---|
296 | Ah yes, you're entirely right on this :-) Typo: adjacent. |
Ah yes, you're entirely right on this :-)
Typo: adjacent.