[IPT] Remove ipt.NumInstScanned statistic

[lukel97]

The NumInstScanned statistic is non-determinstic across multiple identical invocations of LLVM, and leads to noise when trying to diff LLVM statistics with e.g. ./utils/tdiff.py in llvm-test-suite.

My understanding is that it's non-deterministic because the users of IPT's hasSpecialInstructions/isPreceededBySpecialInstruction API aren't deterministic themselves.

This PR removes it and fixes #157598. This is just a small quality-of-life improvement for the ./utils/tdiff.py workflow, but happy to leave the statistic in if others are using it.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Luke Lau (lukel97)

Changes

The NumInstScanned statistic is non-determinstic across multiple identical invocations of LLVM, and leads to noise when trying to diff LLVM statistics with e.g. ./utils/tdiff.py in llvm-test-suite.

My understanding is that it's non-deterministic because the users of IPT's hasSpecialInstructions/isPreceededBySpecialInstruction API aren't deterministic themselves.

This PR removes it and fixes #157598. This is just a small quality-of-life improvement for the ./utils/tdiff.py workflow, but happy to leave the statistic in if others are using it.

---

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

1 Files Affected:

• (modified) llvm/lib/Analysis/InstructionPrecedenceTracking.cpp (-5)

diff --git a/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp b/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp
index a2a9c94c14ae8..8bebd465ff7d3 100644
--- a/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp
+++ b/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp
@@ -19,15 +19,11 @@
 
 #include "llvm/Analysis/InstructionPrecedenceTracking.h"
 #include "llvm/Analysis/ValueTracking.h"
-#include "llvm/ADT/Statistic.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/Support/CommandLine.h"
 
 using namespace llvm;
 
-#define DEBUG_TYPE "ipt"
-STATISTIC(NumInstScanned, "Number of insts scanned while updating ibt");
-
 #ifndef NDEBUG
 static cl::opt<bool> ExpensiveAsserts(
     "ipt-expensive-asserts",
@@ -50,7 +46,6 @@ const Instruction *InstructionPrecedenceTracking::getFirstSpecialInstruction(
   auto [It, Inserted] = FirstSpecialInsts.try_emplace(BB);
   if (Inserted) {
     for (const auto &I : *BB) {
-      NumInstScanned++;
       if (isSpecialInstruction(&I)) {
         It->second = &I;
         break;

[fhahn]

Seems fine, but curious if you know why this is non-deterministic?

[lukel97]

Seems fine, but curious if you know why this is non-deterministic?

My best guess is that there's users like this, where the blocks iterator() might not always be in the same order

void ICFLoopSafetyInfo::computeLoopSafetyInfo(const Loop *CurLoop) {
  assert(CurLoop != nullptr && "CurLoop can't be null");
  ICF.clear();
  MW.clear();
  MayThrow = false;
  // Figure out the fact that at least one block may throw.
  for (const auto &BB : CurLoop->blocks())
    if (ICF.hasICF(&*BB)) {
      MayThrow = true;
      break;
    }

But I haven't confirmed.

[github-actions]

🐧 Linux x64 Test Results

• 186286 tests passed
• 4853 tests skipped

[fhahn]

LGTM, thanks.

It can always be brought back and made deterministic if needed