This CL implements v128.const for each vector type. New operand types
are added to ensure the vector contents can be serialized without LEB
encoding.
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 21599 Build 21599: arc lint + arc unit
Event Timeline
Generally looks good.. but what about some tests?
Should also add support for these to the assembler, or in a follow-up?
I'm always down for more tests. What tests could I add?
Should also add support for these to the assembler, or in a follow-up?
Can you point me to the code that will need to be modified for this?
Assembler is in WebAssemblyAsmParser.cpp
There's some disassembler tests in test/MC/Disassembler/wasm.txt
Instruction selection part looks good. I guess other reviewers can give better feedback on the encoding/disassembling parts.
lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp | ||
---|---|---|
184 | Are these added in order to prevent elements from being LEB-encoded one by one? SIMD immediates do not use LEB encoding? | |
lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp | ||
94 | clang-format this switch-case block and also possibly other modified parts. | |
112 | Fix indentation (I guess it will be fixed anyway if you run clang-format though) |
lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp | ||
---|---|---|
184 | That's correct. The spec says that the vector immediate is actually an array of 16 immediate bytes, i.e. not LEB-encoded. However, ISEL wants to treat v16i8, v8i16, v4i32, etc. all separately rather than converting them all to v16i8 first, so we need these new operand types in order to encode the larger lane values without LEB. |
lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp | ||
---|---|---|
95 | For extra fun template points you could combine this with the FP case below for a single parseImmediate, where you could add something like |
For extra fun template points you could combine this with the FP case below for a single parseImmediate, where you could add something like
if (std::is_floating_point<T>::value) { ... } else { ... }
You can probably even do the conditional statically, although I'm not sure that would be more readable.