[PATCH] D125392: [riscv] Canonicalize vsetvli (vsetvli avl, vtype1) vtype2 transitions

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1165
             // we could merge these two VSETVLI.
-            if (HasSameAVL && CurInfo.hasSameVLMAX(NewInfo)) {
+            // TODO: If we remove this, we get a `vsetvli x0, x0, vtype'
+            // here.  We could simply let this be emitted, then remove
----------------
Makes sense to me to do a post-pass. Though not for this patch.


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1170
+              // WARNING: For correctness, it is essential the contents of VL
+              // and VTYPE stay the same after this instruction.  This
+              // greatly limits the mutation we can legally do here.
----------------
I think I'm getting confused by the comment here - what's "this instruction"? The current MI or the previous VSETVLI?


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1258
+        // from the current state for the new one.  This allows us to
+        // generate 'vsetvli x0, x0, vtype" or possible skip the transition
+        // entirely.
----------------
`possibly`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125392