[riscv] Add strict asserts for VSETVLI insertion algorithm to help catch bugs

[reames]

Summary

This assertion should hold for any reasonable data flow algorithm, but is known not to in several cases today. I'd like to go ahead and land this off-by-default, so that we can collaborate on fixes and have a common definition of success.

[reames]

When I split this out of another patch set, I apparently missed updating the assert. This assert only holds if we encountered an instruction which needs a state, otherwise we have an uninitialized current state. Updated diff forthcoming.

[reames]

Allow uninitialized current state.

[frasercrmck]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1188

printMBBReference might be better since getName returns an empty string if there's no IR BB?

[reames]

Address review comment.

[frasercrmck]

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

1193

Sorry - last minute comment. Maybe an assertion message would help people report issues to us more easily.

[reames]

Address review comment

(If you want particular wording on the assert message, include it in a comment and I'll change before landing.)

[frasercrmck]

LGTM, thanks

[reames]

This has landed. I got the tag wrong in the commit message so the auto-close didn't kick in.

commit f486119ce94573793c1569f1542c09fae74a0d1d
Author: Philip Reames <[e-mail]>
Date: Thu May 5 12:50:47 2022 -0700

[riscv] Add strict asserts for VSETVLI insertion algorithm to help catch bugs

This assertion should hold for any reasonable data flow algorithm, but is known not to in several cases today. I'd like to go ahead and land this off-by-default, so that we can collaborate on fixes and have a common definition of success.

Differential: https://reviews.llvm.org/D125035

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

 
 #define DEBUG_TYPE "riscv-insert-vsetvli"
 #define RISCV_INSERT_VSETVLI_NAME "RISCV Insert VSETVLI pass"
 
 static cl::opt<bool> DisableInsertVSETVLPHIOpt(
     "riscv-disable-insert-vsetvl-phi-opt", cl::init(false), cl::Hidden,
     cl::desc("Disable looking through phis when inserting vsetvlis."));
 
+static cl::opt<bool> UseStrictAsserts(
+    "riscv-insert-vsetvl-strict-asserts", cl::init(false), cl::Hidden,
+    cl::desc("Enable strict assertion checking for the dataflow algorithm"));
+
 namespace {
 
 class VSETVLIInfo {
   union {
     Register AVLReg;
     unsigned AVLImm;
   };
 
for (MachineInstr &MI : MBB) {
     if (MI.isCall() || MI.isInlineAsm() || MI.modifiesRegister(RISCV::VL) ||
         MI.modifiesRegister(RISCV::VTYPE)) {
       CurInfo = VSETVLIInfo::getUnknown();
       PrevVSETVLIMI = nullptr;
     }
 
     // If we reach the end of the block and our current info doesn't match the
     // expected info, insert a vsetvli to correct.
-    if (MI.isTerminator()) {
+    if (!UseStrictAsserts && MI.isTerminator()) {
       const VSETVLIInfo &ExitInfo = BlockInfo[MBB.getNumber()].Exit;
       if (CurInfo.isValid() && ExitInfo.isValid() && !ExitInfo.isUnknown() &&
           CurInfo != ExitInfo) {
         insertVSETVLI(MBB, MI, ExitInfo, CurInfo);
         CurInfo = ExitInfo;
       }
     }
   }
+
+  if (UseStrictAsserts && CurInfo.isValid()) {
+    const auto &Info = BlockInfo[MBB.getNumber()];
+    if (CurInfo != Info.Exit) {
+      LLVM_DEBUG(dbgs() << "in block " << printMBBReference(MBB) << "\n");
+      LLVM_DEBUG(dbgs() << "  begin        state: " << Info.Pred << "\n");
+      LLVM_DEBUG(dbgs() << "  expected end state: " << Info.Exit << "\n");
+      LLVM_DEBUG(dbgs() << "  actual   end state: " << CurInfo << "\n");
+    }
+    assert(CurInfo == Info.Exit &&
+           "InsertVSETVLI dataflow invariant violated");
+  }
 }
 
 bool RISCVInsertVSETVLI::runOnMachineFunction(MachineFunction &MF) {
   // Skip if the vector extension is not enabled.
   const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
   if (!ST.hasVInstructions())
     return false;