[RISCV][InsertVSETVLI] Handle partially transparent instructions in PRE

[reames]

Summary

The PRE implementation was being overly strict when checking to see if a vsetvli was removed in the current block. For instructions which don't use all the fields of VTYPE or VL, we can propagate a changed state past the first instruction with an SEW operand and remove a vsetvli later in the block. We do need to be careful now to ensure that the state convergences before the end of the block or we'd invalidate the cached data flow results.

Taking a step back, we're modeling the effect of the emitVSETVLIs pass which runs just after PRE. This is unfortunate, and makes me think we should probably reevaluate doing the PRE as a post-pass instead of as surgery in the data flow phases. Doing that requires us to get more aggressive about mutating user written vsetvlis which we've tried not to do up to now, but well, maybe it's time? Anyways, that's a thought for the future, not something I'm proposing doing now.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1197

vesetvli -> vsetvli

1201

Why int64_t as opposed to int?

llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll

956–957

opportunity*

[reames]

Address review comments

[reames]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1201

What's the largest number of instructions a basic block can hold? Is it bounded by an i31? It probably is in all practical term, but int64_t felt "safer".

[craig.topper]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1201

I'd imagine you'd run out of virtual register numbers before you got past i31. Unless you had a lot of instructions that didn't write a register.

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

void RISCVInsertVSETVLI::doPRE(MachineBasicBlock &MBB) {
   // Critical edge - TODO: consider splitting?
   if (UnavailablePred->succ_size() != 1)
     return;
 
   // If VL can be less than AVL, then we can't reduce the frequency of exec.
   if (!hasFixedResult(AvailableInfo, ST))
     return;
 
-  // Does it actually let us remove an implicit transition in MBB?
-  bool Found = false;
-  for (auto &MI : MBB) {
-    if (isVectorConfigInstr(MI))
-      return;
-
-    const uint64_t TSFlags = MI.getDesc().TSFlags;
-    if (RISCVII::hasSEWOp(TSFlags)) {
-      if (AvailableInfo != computeInfoForInstr(MI, TSFlags, MRI))
-        return;
-      Found = true;
+  // Model the effect of changing the input state of the block MBB to
+  // AvailableInfo.  We're looking for two issues here; one legality,
+  // one profitability.
+  // 1) If the block doesn't use some of the fields from VL or VTYPE, we
+  //    may hit the end of the block with a different end state.  We can
+  //    not make this change without reflowing later blocks as well.
+  // 2) If we don't actually remove a transition, inserting a vsetvli
+  //    into the predecessor block would be correct, but unprofitable.
+  VSETVLIInfo OldInfo = BlockInfo[MBB.getNumber()].Pred;
+  VSETVLIInfo CurInfo = AvailableInfo;
+  int TransitionsRemoved = 0;
+  for (const MachineInstr &MI : MBB) {
+    const VSETVLIInfo LastInfo = CurInfo;
+    const VSETVLIInfo LastOldInfo = OldInfo;
+    transferBefore(CurInfo, MI);
+    transferBefore(OldInfo, MI);
+    if (CurInfo == LastInfo)
+      TransitionsRemoved++;
+    if (LastOldInfo == OldInfo)
+      TransitionsRemoved--;
+    transferAfter(CurInfo, MI);
+    transferAfter(OldInfo, MI);
+    if (CurInfo == OldInfo)
+      // Convergence.  All transitions after this must match by construction.
       break;
-    }
-  }
-  if (!Found)
+  if (CurInfo != OldInfo || TransitionsRemoved <= 0)
+    // Issues 1 and 2 above
     return;
 
   // Finally, update both data flow state and insert the actual vsetvli.
   // Doing both keeps the code in sync with the dataflow results, which
   // is critical for correctness of phase 3.
-  auto OldInfo = BlockInfo[UnavailablePred->getNumber()].Exit;
+  auto OldExit = BlockInfo[UnavailablePred->getNumber()].Exit;
   LLVM_DEBUG(dbgs() << "PRE VSETVLI from " << MBB.getName() << " to "
                     << UnavailablePred->getName() << " with state "
                     << AvailableInfo << "\n");
   BlockInfo[UnavailablePred->getNumber()].Exit = AvailableInfo;
   BlockInfo[MBB.getNumber()].Pred = AvailableInfo;
 
   // Note there's an implicit assumption here that terminators never use
   // or modify VL or VTYPE.  Also, fallthrough will return end().
   auto InsertPt = UnavailablePred->getFirstInstrTerminator();
   insertVSETVLI(*UnavailablePred, InsertPt,
                 UnavailablePred->findDebugLoc(InsertPt),
-                AvailableInfo, OldInfo);
+                AvailableInfo, OldExit);
 }
 
 static void doUnion(DemandedFields &A, DemandedFields B) {
   A.VLAny |= B.VLAny;
   A.VLZeroness |= B.VLZeroness;
   A.SEW |= B.SEW;
   A.LMUL |= B.LMUL;
   A.SEWLMULRatio |= B.SEWLMULRatio;

llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll

if:
   %a = call <vscale x 2 x i32> @llvm.riscv.vle.nxv2i32(<vscale x 2 x i32> undef, <vscale x 2 x i32>* %x, i64 2)
   br label %if.end
 
 if.end:
   %d = phi <vscale x 2 x i32> [ %a, %if ], [ %c, %entry ]
   %e = insertelement <vscale x 2 x i32> %d, i32 0, i32 0
   ret <vscale x 2 x i32> %e
 }
 
-; This case demonstrates a PRE oppurtunity where the first instruction
-; in the block doesn't require a state transition.  Essentially, we need
-; to FRE the transition to the start of the block, and *then* PRE it.
+; This case demonstrates a PRE case where the first instruction in the block
+; doesn't require a state transition.
 define void @pre_over_vle(ptr %A) {
 ; CHECK-LABEL: pre_over_vle:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    li a1, 100
+; CHECK-NEXT:    vsetivli zero, 2, e32, mf2, ta, ma
 ; CHECK-NEXT:  .LBB22_1: # %vector.body
 ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
-; CHECK-NEXT:    vsetivli zero, 2, e32, mf2, ta, ma
 ; CHECK-NEXT:    vle8.v v8, (a0)
 ; CHECK-NEXT:    vsext.vf4 v9, v8
 ; CHECK-NEXT:    vse32.v v9, (a0)
 ; CHECK-NEXT:    addi a1, a1, -1
 ; CHECK-NEXT:    addi a0, a0, 8
 ; CHECK-NEXT:    bnez a1, .LBB22_1
 ; CHECK-NEXT:  # %bb.2: # %exit
 ; CHECK-NEXT:    ret