[PATCH] D107957: [LegalizeTypes][VP] Add splitting support for binary VP ops

Wed Sep 1 09:34:43 PDT 2021

craig.topper added inline comments.

================
Comment at: llvm/test/CodeGen/RISCV/rvv/vadd-vp.ll:1627
+; FIXME: The first vadd.vi should be able to infer that its AVL is equivalent to VLMAX.
+; FIXME: The second vsetvli is incorrectly preserving the VL on RV64, rather
+; than setting it to 0 as in RV32.
----------------
frasercrmck wrote:
> frasercrmck wrote:
> > @craig.topper, do you have any thoughts about this issue? I don't think it's specific to this patch but rather is just exposed by it. We could probably generated a reduced test case on the current `main`.
> FWIW this is a problem in `main` but only in the sense that we //could// create something like
> 
> ```
>     %0:gpr = COPY $x0
>     dead %1:gpr = PseudoVSETVLI %0, 89, implicit-def $vl, implicit-def $vtype
> ```
> 
> which can then be register-coalesced into `PseudoVSETVLI $x0`. We tend not to generate this sort of thing. It's just that in this example the `COPY $x0` isn't immediately apparent during instruction selection due to the pseudo instructions.
> 
> I couldn't find a way of stopping the coalescer do its job here. It's legal from an ISA to do it, but it's not from a semantics point-of-view. It's almost like we need two different `PseudoVSETVLI` instructions: one that takes `x0` and one that doesn't.
I'm looking into this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107957