This commit is a follow-on to c2c2e9119e73, using the Vec records introduced
in that commit in the rest of the SIMD instruction definitions. Also removes
unnecessary types in output patterns.
Details
- Reviewers
aheejin - Commits
- rG44ee14f993ff: [WebAssembly][NFC] Finish cleaning up SIMD tablegen
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nice!
llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td | ||
---|---|---|
63 | Can we do this? (Other places too) | |
509 | Does not using LaneIdx32 change anything? (I guess LaneIdx has range checks but this doesn't?) (By the way it looks it should be LaneIdx16) | |
526–530 | ||
722 | How about defining something like defvar IntVecs = [I8x16, I16x8, I32x4, I64x2]; at the top and use it elsewhere? | |
725 | Any reason SIMDBitwise does not use SIMDBinary anymore? | |
736–740 | Any reason NOT does not use SIMDUnary? | |
1121–1130 | Is it subject to change? | |
1146–1158 | Any reason this does not use SIMDConvert? |
- Reuse vt as suggested
- Add IntVecs as suggested
llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td | ||
---|---|---|
63 | Yes, it turns out we can! | |
509 | If I understand correctly, using LaneIdx32 in the input pattern is sufficient to apply the range checks, so it's not needed in the output pattern as well. (However, imm is still needed because it makes the resulting MachineOperand an immediate rather than a register.) Shuffle uses LaneIdx32 because the indices can refer to bytes in either of the two input vectors. | |
722 | Good idea, done. | |
725 | The previous version used SIMDBitwise created four different version of the same instruction when all we really needed was one instruction with four different patterns. Since it turns out we can use foreach in a multiclass to generate Pats and use NAME to refer to the instruction in those Pats, this seemed like a good way to get rid of the redundant instruction definitions. This also lets SIMDBinary get rid of the prefix argument since SIMDBinary no longer needs to generate instructions that start with "v128." | |
736–740 | Similar to the reasons for the previous change. We don't need four different versions of NOT and this lets SIMDUnary get rid of the prefix argument. | |
1121–1130 | These opcodes are not part of a consistent design, but rather chosen from the available opcode space to get prototypes done in V8 as quickly as possible. I'll be a little sad if the opcodes we end up shipping don't have a consistent arrangement, but we haven't discussed whether we are going to do another change yet, so I don't know whether this will change. | |
1146–1158 | Yes, these narrowing operations take two operands but SIMDConvert instructions have just one operand. |
Thanks for the explanation! LGTM % the alignment nit
llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td | ||
---|---|---|
526–530 | ping |
Can we do this? (Other places too)