[PATCH] D92228: [RISCV] Add MIR tests exposing missed InstAliases
Roger Ferrer Ibanez via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 30 13:44:16 PST 2020
rogfer01 added a comment.
Hi all,
a couple of comments.
1. First regarding the non-matched aliases: we also observed this issue when working on that and we deployed a bit of a hack. We added a new attribute `AllowsNoRegister`
// AllowsNoRegister - Specify that instructions may use NoRegister (0) instead
// of a physical register for an operand of this register class.
bit AllowsNoRegister = 0;
And then we extended `VMV0` register class with this attribute
def VMV0 : RegisterClass<"RISCV", VMaskVT.types, 64, (add V0)> {
let Size = 64;
let AllowsNoRegister = 1;
}
and then we updated `matchAliasCondition` in `MCInstPrinter.cpp` to look like this (after having implemented some needed plumbing to bring the bit where used)
case AliasPatternCond::K_RegClass:
// Operand must be a register in this class. Value is a register class id.
return Opnd.isReg() && (MRI.getRegClass(C.Value).contains(Opnd.getReg()) ||
(MRI.getRegClass(C.Value).allowsNoRegister() &&
Opnd.getReg() == 0));
but this might feel a bit hackish, so I'm not suggesting this approach (unless there is a general sentiment that it is sensible).
2. Regarding `@earlyclobber`, yes it is not ideal as it is now. But it doesn't only impact masking (though perhaps that is the clearest case).
Unless the ISA has changed (and I admit I might be a bit outdated here) my understanding is that we still have cases where `@earlyclobber` will be needed and it will still fall short.
For instance, mixed widenings such as `vwadd.wv` are complex to model because iirc an instruction like `vwadd.wv v2, v2, v1` is fine while `vwadd.wv v2, v2, v3` is not. I understand that using `@earlyclobber` a thing like `vwadd.wv v4, v2, v1` is the best we can get.
I asked about this here http://lists.llvm.org/pipermail/llvm-dev/2020-May/141383.html
One of the suggestions made in that thread is teaching RA new tricks, which I don't feel qualified to do :) (I envision that if RA were able to dynamically limit the valid registers, as it assigns registers to operands, by virtue of some target-specific constraints, then, perhaps, this would be doable. But tbh I have no idea such approach is even feasible in the current RA. If we could do that we could also solve the case where `v0` can't be used as the destination of masked instructions).
The other suggestion is to amend the instructions after RA in a specific pass. Seems doable at first but I am afraid this is going to be testing hell just to make sure no instruction manages to escape from that pass incorrectly.
I personally don't oppose duplicating all the instructions between mask and unmasked if this simplifies things. Just let's be conscious that, for instructions with a mask, we would go from 7 pseudos per actual instruction to 14. Perhaps the impact is not that noticeable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92228/new/
https://reviews.llvm.org/D92228
More information about the llvm-commits
mailing list