[PATCH] D126893: [RISCV] simplify emitVSETVLIs handling of vsetvli xN, phi(), vtype case [NFC]

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: frasercrmck, craig.topper, kito-cheng.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, bollu, simoncook, johnrusso, rbar, asb, hiraditya, arichardson, mcrosier.
Herald added a project: All.
reames requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

This is possibly somewhat subjective, but having an explicitly named flag to track the property required and code structure that more closely matches phase 1/2 of the dataflow seems much easier to read.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126893

Files:
  llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp


Index: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -1088,7 +1088,8 @@
 }
 
 void RISCVInsertVSETVLI::emitVSETVLIs(MachineBasicBlock &MBB) {
-  VSETVLIInfo CurInfo;
+  VSETVLIInfo CurInfo = BlockInfo[MBB.getNumber()].Pred;
+  bool PrefixTransparent = true;
   for (MachineInstr &MI : MBB) {
     // If this is an explicit VSETVLI or VSETIVLI, update our state.
     if (isVectorConfigInstr(MI)) {
@@ -1099,6 +1100,7 @@
       MI.getOperand(3).setIsDead(false);
       MI.getOperand(4).setIsDead(false);
       CurInfo = getInfoForVSETVLI(MI);
+      PrefixTransparent = false;
       continue;
     }
 
@@ -1118,33 +1120,20 @@
       MI.addOperand(MachineOperand::CreateReg(RISCV::VTYPE, /*isDef*/ false,
                                               /*isImp*/ true));
 
-      if (!CurInfo.isValid()) {
-        // We haven't found any vector instructions or VL/VTYPE changes yet,
-        // use the predecessor information.
-        CurInfo = BlockInfo[MBB.getNumber()].Pred;
-        assert(CurInfo.isValid() && "Expected a valid predecessor state.");
-        if (needVSETVLI(MI, NewInfo, CurInfo)) {
-          // If this is the first implicit state change, and the state change
-          // requested can be proven to produce the same register contents, we
-          // can skip emitting the actual state change and continue as if we
-          // had since we know the GPR result of the implicit state change
-          // wouldn't be used and VL/VTYPE registers are correct.  Note that
-          // we *do* need to model the state as if it changed as while the
-          // register contents are unchanged, the abstract model can change.
-          if (needVSETVLIPHI(NewInfo, MBB))
-            insertVSETVLI(MBB, MI, NewInfo, CurInfo);
-          CurInfo = NewInfo;
-        }
-      } else {
-        // If this instruction isn't compatible with the previous VL/VTYPE
-        // we need to insert a VSETVLI.
-        // NOTE: We can't use predecessor information for the store. We must
-        // treat it the same as the first phase so that we produce the correct
-        // vl/vtype for succesor blocks.
-        if (needVSETVLI(MI, NewInfo, CurInfo)) {
+      // If this instruction isn't compatible with the previous VL/VTYPE
+      // we need to update the abstract state, and possibly insert a VSETVLI.
+      if (needVSETVLI(MI, NewInfo, CurInfo)) {
+        // If this is the first implicit state change, and the state change
+        // requested can be proven to produce the same register contents, we
+        // can skip emitting the actual state change and continue as if we
+        // had since we know the GPR result of the implicit state change
+        // wouldn't be used and VL/VTYPE registers are correct.  Note that
+        // we *do* need to model the state as if it changed as while the
+        // register contents are unchanged, the abstract model can change.
+        if (!PrefixTransparent || needVSETVLIPHI(NewInfo, MBB))
           insertVSETVLI(MBB, MI, NewInfo, CurInfo);
-          CurInfo = NewInfo;
-        }
+        PrefixTransparent = true;
+        CurInfo = NewInfo;
       }
     }
 
@@ -1153,6 +1142,7 @@
     if (MI.isCall() || MI.isInlineAsm() || MI.modifiesRegister(RISCV::VL) ||
         MI.modifiesRegister(RISCV::VTYPE)) {
       CurInfo = VSETVLIInfo::getUnknown();
+      PrefixTransparent = false;
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126893.433776.patch
Type: text/x-patch
Size: 3555 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220602/2f319fe2/attachment.bin>