[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;
----------------
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)`.


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

https://reviews.llvm.org/D92679