[PATCH] D92679: [RISCV] Add a pass to remove duplicate VSETVLI instructions in a basic block.

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVCleanupVSETVLI.cpp:85
+    // Does this VSETVLI use the same AVL register and VTYPE immediate?
+    if (PrevAVLReg != AVLReg || PrevVTYPEImm != VTYPEImm) {
+      PrevVSETVLI = &MI;
----------------
rogfer01 wrote:
> craig.topper wrote:
> > Thinking about this more, this isn't correct if AVL is X0. I think we would need that the that they both have a non-X0 Def. Or that the one we're removing has an X0 def.
> Right.
> 
> If AVL is `x0` we have the following cases
> 
> ```
> vsetvli rdest0, x0, sew,lmul # rdest0 != x0
> vsetvli rdest1, x0, sew,lmul  # rdest1 != x0 → redundant as vl stays the same and rdest1 == rdest0. OK to remove if rdest1 is dead
> ```
> 
> ```
> vsetvli rdest, x0, sew,lmul # rdest != x0
> vsetvli x0, x0, sew,lmul  # redundant, nothing has changed at all, OK to remove
> ```
> 
> As you already pointed it out:
> 
> ```
> # earlier we set vtype ← sew0,lmul0,...
> vsetvli x0, x0, sew1,lmul1 # (A) rdest != x0. # This could reduce vl (because  sew1 * lmul1 < sew0 * lmul0)
> vsetvli rdest, x0, sew1,lmul1  # (B) We  set vl to to vlmax(sew1,lmul1)
> ```
> 
> Because we don't really know the value of `vl` at `(A)` and given §6.2 of the 0.9 spec (which gives some additional freedom to the new value when the AVL is larger than VLMAX), I think the safe thing to do here is not to remove `(B)`.
Well, I forgot one case:

```
vsetvli x0, x0, sew,lmul 
vsetvli x0, x0, sew,lmul  # redundant, OK to remove
```


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

https://reviews.llvm.org/D92679