[PATCH] D92228: [RISCV] Add MIR tests exposing missed InstAliases

Thu Dec 10 09:24:45 PST 2020

evandro added a comment.

In D92228#2424007 <https://reviews.llvm.org/D92228#2424007>, @rogfer01 wrote:

> 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.

IMO, this last suggestion is no different from the first one: new RA tricks.  Like you, I don't feel qualified, but I also feel that it's a rabbit hole too, especially the last suggestion, in terms of maintenance.

In D92228#2444352 <https://reviews.llvm.org/D92228#2444352>, @HsiangKai wrote:

> Although some cases are unavoidable to use `@earlyclobber` to conform the instruction constraints, it is still beneficial to model the pseudo instructions using different classes for mask and unmasked instructions. It could avoid unnecessary copies between vector registers for `@earlyclobber` constraint. Vector copy should be expensive and it hurts performance of vector code.

I agree, this is the right thing to do.

================
Comment at: llvm/test/CodeGen/MIR/RISCV/aliases-v.mir:34
+name:            vnot_mask_1
+body:             |
+  bb.0:
----------------
Is there any reason why the bodies are outside the definitions?  It's a bit confusing...

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92228/new/

https://reviews.llvm.org/D92228