[PATCH] D89449: [RISCV] Initial infrastructure for code generation of the RISC-V V-extension

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoPseudoV.td:145
+multiclass pseudo_binary_v_vv_vx_vi<Operand imm_type = simm5,
+                                    bit force_earlyclobber = 0> {
+  let mayLoad = 0, mayStore = 0, hasSideEffects = 0, usesCustomInserter = 1 in
----------------
rogfer01 wrote:
> frasercrmck wrote:
> > I notice that the RFC mentions using `early-clobber` constraints but don't see it being used here. From the RFC:
> > 
> > ```
> > early-clobber %2:vrm2 = PseudoVADD_VV_M2 %3:vrm2(tied-def 0), %0:vrm2, %1:vrm2,$noreg, $x0, 32, implicit $vl, implicit $vtype
> > 
> > (If you wonder about the early-clobber it is needed to fulfill some constraints between sources and destination registers under lmul>1)
> > ```
> > 
> > I ask because I'm concerned about the use of `tied` and `early-clobber` on the same operand: it is a special-case in SlotIndexes (as once explained in the [[ http://lists.llvm.org/pipermail/llvm-dev/2017-February/110492.html | mailing lists ]]) and I've seen issues with this on another target I was working on, where LLVM forgets about this special case in several places and generates wrong code (subregister lanes are incorrectly deemed to be `undef`). I worry we're going to see really hard-to-track bugs a few months down the line when trying to compile more complex programs.
> > 
> > Is `early-clobber` really needed? Perhaps you could explain which constraints under lmul>1 are fulfilled by using this?
> Hi, apologies I wasn't clear enough with this aspect of the proposal.
> 
> `early-clobber` is in practice only relevant for widenings and narrowings (and a few other instructions). That is the reason why it is not in this very first patch. In the particular case of widenings and narrowings, we cannot have a def operand and a use operand where their `sew` is different and their actual vector registers overlap (under some conditions).
> 
> For instance `vwadd.vv v2, v1, v2` is not valid (the rule is a bit obscure as I understand `vwadd.vv v2, v1, v3` might be valid, see https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#52-vector-operands). The simplest way to avoid this issue was using `early-clobber`.
> 
> If we don't use `early-clobber` then I understand we need to amend somehow the instructions after RA. Perhaps it is possible to let RA know what registers are still feasible as it goes allocating them? (I have not looked into that, tbh)
> 
> There are some cases where `early-clobber` may be too strict. I asked the list whether there is a way to model something more than what `early-clobber` does (for operands that have the same EEW as the destination, consider `vwadd.wv`)  in http://lists.llvm.org/pipermail/llvm-dev/2020-May/141383.html but apparently there is no straightforward solution at the moment.
Hi Roger, thanks for the explanation. I see where you're coming from and why `early-clobber` is needed. I think we should just go with `early-clobber` for now and fix the bugs that come our way, or improve LLVM in line with the question you asked on the list.

And on that note, it sounds like what we need is something like `let Constraints = "$src != $dst"`: it sounds like a register allocation thing rather than a liveness thing, doesn't it?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D89449