Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Remove -riscv-insert-vsetvl-strict-asserts flag #90171

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Apr 26, 2024

This flag has been enabled by default for almost two years now since 1f06398, and at this stage we probably shouldn't be falling back to the fixups.

This removes the flag so we always perform the assertion, as well as making sure that CurInfo is always valid on exit: We shouldn't leave emitVSETVLIs with an uninitialized VSETVLIInfo.

This flag has been enabled by default for almost two years now since 1f06398, and at this stage we probably shouldn't be falling back to the fixups.

This removes the flag so we always perform the assertion, as well as making sure that CurInfo is always valid on exit: We shouldn't leave emitVSETVLIs with an unitialized VSETVLIInfo.
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 26, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

Changes

This flag has been enabled by default for almost two years now since 1f06398, and at this stage we probably shouldn't be falling back to the fixups.

This removes the flag so we always perform the assertion, as well as making sure that CurInfo is always valid on exit: We shouldn't leave emitVSETVLIs with an unitialized VSETVLIInfo.


Full diff: https://github.com/llvm/llvm-project/pull/90171.diff

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp (+8-29)
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index c40b9031543fe2..90fa0920c49283 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -45,10 +45,6 @@ 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(true), cl::Hidden,
-    cl::desc("Enable strict assertion checking for the dataflow algorithm"));
-
 namespace {
 
 static unsigned getVLOpNum(const MachineInstr &MI) {
@@ -1430,32 +1426,15 @@ void RISCVInsertVSETVLI::emitVSETVLIs(MachineBasicBlock &MBB) {
     transferAfter(CurInfo, MI);
   }
 
-  // If we reach the end of the block and our current info doesn't match the
-  // expected info, insert a vsetvli to correct.
-  if (!UseStrictAsserts) {
-    const VSETVLIInfo &ExitInfo = BlockInfo[MBB.getNumber()].Exit;
-    if (CurInfo.isValid() && ExitInfo.isValid() && !ExitInfo.isUnknown() &&
-        CurInfo != ExitInfo) {
-      // Note there's an implicit assumption here that terminators never use
-      // or modify VL or VTYPE.  Also, fallthrough will return end().
-      auto InsertPt = MBB.getFirstInstrTerminator();
-      insertVSETVLI(MBB, InsertPt, MBB.findDebugLoc(InsertPt), 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");
+  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");
 }
 
 /// Perform simple partial redundancy elimination of the VSETVLI instructions

Copy link

github-actions bot commented Apr 26, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@lukel97 lukel97 merged commit 7faf343 into llvm:main Apr 30, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants