This is the family of vector instructions that combine all the lanes
in their input vector(s), and output a value in one or two GPRs.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/ARM/ARMInstrMVE.td | ||
---|---|---|
331 | There seems to be a lot of repetition here, could some of these classes be combined? For example most bits of t2VMINNM, t2VMINV, t2VMAXNMV and t2VMAX are the same. Could we also use multiclasses to avoid repeating the list of types for each of the MIN and MAX versions of these instructions? Also, is the a pattern to whether these names end in "V" which I've missed, or could they be made more consistent? | |
346 | What is this comment adding? | |
451 | Again, the t2VMLADAV and t2VMLSDAV classes share most of their bits, and could be merged. | |
llvm/test/MC/ARM/mve-reductions.s | ||
14 | We could avoid repeating all of these check lines by either adding a new check-prefix shared between the two runs, or moving the FP instructions to a separate file. As noted in previous reviews, we should also be checking the error message when the FP instructions are rejected. |
llvm/lib/Target/ARM/ARMInstrMVE.td | ||
---|---|---|
271 | As a corollary to what Oliver was saying. The VADDV (or in VADDVs8no_acc) doesn't really take a Rda_src, I believe. At least the structure you would match from llvm would not. There's a perfectly good chance it's possible and I just don't know how to represent that in a pattern. But it seems more natural to make sure that VADDVs*no_acc don't require a Rda_src. When I looked at this downstream, I just ended up splitting this into two classes. |
llvm/lib/Target/ARM/ARMInstrMVE.td | ||
---|---|---|
271 |
This also pertains to VMLADAV, VMLSDAV, etc, right? |
- Fixed naming inconsistencies
- Removed comments about undefined instructions
- Factored out common parts of:
- VMINV and VMAXV
- VMINNMV and VMAXNMV
- VMLADAV and VMLSDAV
- VMLALDAV, VMLSLDAV (includes VRMLSLDAVH) and VRMLALDAVH
- Fixed inputs and constraints of non-accumulating instructions
- Split the assembler test into integer and floating point parts
llvm/lib/Target/ARM/ARMInstrMVE.td | ||
---|---|---|
272 | This class is only instantiated once with each value of A, so I think it would be clearer to make these parameters and move the values down to the multiclass below. | |
421 | Again, this is quite hard to follow, I think it would be better to move the operand lists down to the multiclass. |
llvm/lib/Target/ARM/ARMInstrMVE.td | ||
---|---|---|
421 | But this would mean duplicating everything (i.e., input operands and constraints) 4 times for each case. And 6 times for t2VMLALDAVBase. Wouldn't it be better to create separate subclasses for accumulating and non-accumulating versions? |
As a corollary to what Oliver was saying. The VADDV (or in VADDVs8no_acc) doesn't really take a Rda_src, I believe. At least the structure you would match from llvm would not. There's a perfectly good chance it's possible and I just don't know how to represent that in a pattern. But it seems more natural to make sure that VADDVs*no_acc don't require a Rda_src.
When I looked at this downstream, I just ended up splitting this into two classes.