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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:159
 
+  bool hasSameSEW(const VSETVLIInfo &Other) const {
+    assert(isValid() && Other.isValid() &&
----------------
Is this used?


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:206
 
+  bool hasSamePolicy(const VSETVLIInfo &Other) const {
+    assert(isValid() && Other.isValid() &&
----------------
Is this used?


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:259
 
+    // 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
----------------
 behaves -> behaviors?


================
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.
----------------
"all two" -> both?


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:460
 
+bool IsScalarMoveInstr(const MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
Function names should be lowercase.

And this function should be `static`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116307