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

[GISel] Add debug counter to force sdag fallback #78257

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jan 16, 2024

Add a debug counter that allows forcing an sdag fallback after a certain number of functions.

The intended use-case is to bisect which function gets miscompiled by global isel using -debug-counter=globalisel-count=N (in cases where sdag doesn't also miscompile it, of course).

The "falling back" debug line is printed unconditionally, because irtranslator is extremely spammy, so using LLVM_DEBUG is not viable.

Add a debug counter that allows forcing an sdag fallback after
a certain number of functions.

The intended use-case is to bisect which function gets miscompiled
by global isel using `-debug-counter=globalisel-count=N` (in cases
where sdag doesn't also miscompile it, of course).

The "falling back" debug line is printed unconditionally, because
irtranslator is extremely spammy, so using LLVM_DEBUG is not
helpful.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 16, 2024

@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-llvm-globalisel

Author: Nikita Popov (nikic)

Changes

Add a debug counter that allows forcing an sdag fallback after a certain number of functions.

The intended use-case is to bisect which function gets miscompiled by global isel using -debug-counter=globalisel-count=N (in cases where sdag doesn't also miscompile it, of course).

The "falling back" debug line is printed unconditionally, because irtranslator is extremely spammy, so using LLVM_DEBUG is not viable.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp (+10)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/counter-fallback.ll (+25)
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 662de0f3fe0e5e8..632bb9eb7360a0c 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -76,6 +76,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/DebugCounter.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
@@ -96,6 +97,9 @@
 
 using namespace llvm;
 
+DEBUG_COUNTER(GlobalISelCounter, "globalisel",
+              "Controls whether to select function with GlobalISel");
+
 static cl::opt<bool>
     EnableCSEInIRTranslator("enable-cse-in-irtranslator",
                             cl::desc("Should enable CSE in irtranslator"),
@@ -3701,6 +3705,12 @@ bool IRTranslator::runOnMachineFunction(MachineFunction &CurMF) {
   // Make our arguments/constants entry block fallthrough to the IR entry block.
   EntryBB->addSuccessor(&getMBB(F.front()));
 
+  if (!DebugCounter::shouldExecute(GlobalISelCounter)) {
+    dbgs() << "Falling back for function " << CurMF.getName() << "\n";
+    CurMF.getProperties().set(MachineFunctionProperties::Property::FailedISel);
+    return false;
+  }
+
   if (CLI->fallBackToDAGISel(*MF)) {
     OptimizationRemarkMissed R("gisel-irtranslator", "GISelFailure",
                                F.getSubprogram(), &F.getEntryBlock());
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/counter-fallback.ll b/llvm/test/CodeGen/AArch64/GlobalISel/counter-fallback.ll
new file mode 100644
index 000000000000000..9a39b5a2ad7a927
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/counter-fallback.ll
@@ -0,0 +1,25 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc -mtriple=aarch64-- -global-isel -global-isel-abort=0 -debug-counter=globalisel-count=1 %s -o - 2>/dev/null | FileCheck %s
+; RUN: llc -mtriple=aarch64-- -global-isel -global-isel-abort=0 -debug-counter=globalisel-count=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=DEBUG
+
+; REQUIRES: asserts
+
+; DEBUG-NOT: Falling back for function test1
+; DEBUG: Falling back for function test2
+
+define i32 @test1(i32 %x) {
+; CHECK-LABEL: test1:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ret
+  ret i32 %x
+}
+
+define i32 @test2(i32 %x) {
+; CHECK-LABEL: test2:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ret
+  ret i32 %x
+}
+
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; DEBUG: {{.*}}

@aemerson
Copy link
Contributor

When I did the daemon based bisector a few years back I had to put the fallback trigger in the InstructionSelect pass, since it's possible to fall back naturally all the way through the pipeline. That is if you want to trigger after exactly N GISel functions compiled.

@nikic
Copy link
Contributor Author

nikic commented Jan 16, 2024

When I did the daemon based bisector a few years back I had to put the fallback trigger in the InstructionSelect pass, since it's possible to fall back naturally all the way through the pipeline. That is if you want to trigger after exactly N GISel functions compiled.

So in the current implementation, we would fall back after N functions (regardless of whether they would have fallen back anyway), while doing it in InstructionSelect means we would fall back after N functions that would have otherwise succeeded GlobalISel, right?

I don't really have a strong opinion on either of those choices -- I think for bisection purposes it wouldn't really matter.

@aemerson
Copy link
Contributor

Right, it will still work but the debug counter will be a bit misleading if you're expecting a codegen difference between N and N+1 and the N+1th iteration. For that reason I'd recommend putting it just before here in InstructionSelect.cpp:

  // Determine if there are any calls in this machine function. Ported from
  // SelectionDAG.
  MachineFrameInfo &MFI = MF.getFrameInfo();

@nikic
Copy link
Contributor Author

nikic commented Jan 16, 2024

Okay, done!

@nikic nikic merged commit 435bcea into llvm:main Jan 17, 2024
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