[PATCH] D116307: [RISCV] Teach VSETVLInsert to eliminate redundant vsetvli for vmv.s.x and vfmv.s.f.

[Jianjian Guan via Phabricator via llvm-commits]

jacquesguan added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:159
 
+  bool hasSameSEW(const VSETVLIInfo &Other) const {
+    assert(isValid() && Other.isValid() &&
----------------
craig.topper wrote:
> Is this used?
Done, now use the SEW and policy instead of VTYPE in the condition, because these instructions ignore the LMUL.


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:206
 
+  bool hasSamePolicy(const VSETVLIInfo &Other) const {
+    assert(isValid() && Other.isValid() &&
----------------
craig.topper wrote:
> Is this used?
Done, now use the SEW and policy instead of VTYPE in the condition, because these instructions ignore the LMUL.


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:260
+    // For vmv.s.x and vfmv.s.f, there is only two behaves, VL = 0 and VL > 0.
+    // So it's compatible when we could make sure that all two VL be the same
+    // situation.
----------------
craig.topper wrote:
> "all two" -> both?
Done


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:460
 
+bool IsScalarMoveInstr(const MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
craig.topper wrote:
> Function names should be lowercase.
> 
> And this function should be `static`
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116307