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

[Instrumentation] Remove -pgo-instr-old-cfg-hashing #77357

Merged

Conversation

kazutakahirata
Copy link
Contributor

It's been more than 3 years since -pgo-instr-old-cfg-hashing was
introduced by:

commit 120e66b
Author: Hiroshi Yamauchi yamauchi@google.com
Date: Tue Jul 28 10:09:49 2020 -0700

I don't think anyone really cares about the ability to use the old CFG
hashing at this point.

It's been more than 3 years since -pgo-instr-old-cfg-hashing was
introduced by:

  commit 120e66b
  Author: Hiroshi Yamauchi <yamauchi@google.com>
  Date:   Tue Jul 28 10:09:49 2020 -0700

I don't think anyone really cares about the ability to use the old CFG
hashing at this point.
@llvmbot llvmbot added PGO Profile Guided Optimizations llvm:transforms labels Jan 8, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 8, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

It's been more than 3 years since -pgo-instr-old-cfg-hashing was
introduced by:

commit 120e66b
Author: Hiroshi Yamauchi <yamauchi@google.com>
Date: Tue Jul 28 10:09:49 2020 -0700

I don't think anyone really cares about the ability to use the old CFG
hashing at this point.


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

3 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (+18-33)
  • (modified) llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext (-29)
  • (modified) llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll (-4)
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 3a57709c4e8b7f..6b95c7028d93e9 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -330,10 +330,6 @@ extern cl::opt<std::string> ViewBlockFreqFuncName;
 extern cl::opt<InstrProfCorrelator::ProfCorrelatorKind> ProfileCorrelate;
 } // namespace llvm
 
-static cl::opt<bool>
-    PGOOldCFGHashing("pgo-instr-old-cfg-hashing", cl::init(false), cl::Hidden,
-                     cl::desc("Use the old CFG function hashing"));
-
 // Return a string describing the branch condition that can be
 // used in static branch probability heuristics:
 static std::string getBranchCondString(Instruction *TI) {
@@ -635,34 +631,25 @@ void FuncPGOInstrumentation<Edge, BBInfo>::computeCFGHash() {
   JC.update(Indexes);
 
   JamCRC JCH;
-  if (PGOOldCFGHashing) {
-    // Hash format for context sensitive profile. Reserve 4 bits for other
-    // information.
-    FunctionHash = (uint64_t)SIVisitor.getNumOfSelectInsts() << 56 |
-                   (uint64_t)ValueSites[IPVK_IndirectCallTarget].size() << 48 |
-                   //(uint64_t)ValueSites[IPVK_MemOPSize].size() << 40 |
-                   (uint64_t)MST.numEdges() << 32 | JC.getCRC();
+  // The higher 32 bits.
+  auto updateJCH = [&JCH](uint64_t Num) {
+    uint8_t Data[8];
+    support::endian::write64le(Data, Num);
+    JCH.update(Data);
+  };
+  updateJCH((uint64_t)SIVisitor.getNumOfSelectInsts());
+  updateJCH((uint64_t)ValueSites[IPVK_IndirectCallTarget].size());
+  updateJCH((uint64_t)ValueSites[IPVK_MemOPSize].size());
+  if (BCI) {
+    updateJCH(BCI->getInstrumentedBlocksHash());
   } else {
-    // The higher 32 bits.
-    auto updateJCH = [&JCH](uint64_t Num) {
-      uint8_t Data[8];
-      support::endian::write64le(Data, Num);
-      JCH.update(Data);
-    };
-    updateJCH((uint64_t)SIVisitor.getNumOfSelectInsts());
-    updateJCH((uint64_t)ValueSites[IPVK_IndirectCallTarget].size());
-    updateJCH((uint64_t)ValueSites[IPVK_MemOPSize].size());
-    if (BCI) {
-      updateJCH(BCI->getInstrumentedBlocksHash());
-    } else {
-      updateJCH((uint64_t)MST.numEdges());
-    }
-
-    // Hash format for context sensitive profile. Reserve 4 bits for other
-    // information.
-    FunctionHash = (((uint64_t)JCH.getCRC()) << 28) + JC.getCRC();
+    updateJCH((uint64_t)MST.numEdges());
   }
 
+  // Hash format for context sensitive profile. Reserve 4 bits for other
+  // information.
+  FunctionHash = (((uint64_t)JCH.getCRC()) << 28) + JC.getCRC();
+
   // Reserve bit 60-63 for other information purpose.
   FunctionHash &= 0x0FFFFFFFFFFFFFFF;
   if (IsCS)
@@ -672,10 +659,8 @@ void FuncPGOInstrumentation<Edge, BBInfo>::computeCFGHash() {
                     << ", Selects = " << SIVisitor.getNumOfSelectInsts()
                     << ", Edges = " << MST.numEdges() << ", ICSites = "
                     << ValueSites[IPVK_IndirectCallTarget].size());
-  if (!PGOOldCFGHashing) {
-    LLVM_DEBUG(dbgs() << ", Memops = " << ValueSites[IPVK_MemOPSize].size()
-                      << ", High32 CRC = " << JCH.getCRC());
-  }
+  LLVM_DEBUG(dbgs() << ", Memops = " << ValueSites[IPVK_MemOPSize].size()
+                    << ", High32 CRC = " << JCH.getCRC());
   LLVM_DEBUG(dbgs() << ", Hash = " << FunctionHash << "\n";);
 
   if (PGOTraceFuncHash != "-" && F.getName().contains(PGOTraceFuncHash))
diff --git a/llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext b/llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
index 77f8d5a5ade3b1..1db6cfb445c27e 100644
--- a/llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
+++ b/llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
@@ -9,16 +9,6 @@ _Z3fooi
 18
 12
 
-# For -pgo-instr-old-cfg-hashing=true
-_Z3fooi
-# Func Hash:
-72057606922829823
-# Num Counters:
-2
-# Counter Values:
-18
-6
-
 _Z3fooi
 # Func Hash:
 12884901887
@@ -36,16 +26,6 @@ _Z3bari
 0
 0
 
-# For -pgo-instr-old-cfg-hashing=true
-_Z3bari
-# Func Hash:
-72057606922829823
-# Num Counters:
-2
-# Counter Values:
-0
-0
-
 _Z4m2f1v
 # Func Hash:
 742261418966908927
@@ -53,12 +33,3 @@ _Z4m2f1v
 1
 # Counter Values:
 1
-
-# For -pgo-instr-old-cfg-hashing=true
-_Z4m2f1v
-# Func Hash:
-12884901887
-# Num Counters:
-1
-# Counter Values:
-1
diff --git a/llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll b/llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
index 6a7838080b1dc5..768412603a846c 100644
--- a/llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
+++ b/llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
@@ -1,6 +1,5 @@
 ; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
-; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -29,9 +28,6 @@ entry:
 ; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i
 ; CHECK-SAME: !prof ![[BW:[0-9]+]]
 ; CHECK: ![[BW]] = !{!"branch_weights", i32 12, i32 6}
-; CHECKOLDHASH: %mul.i = select i1 %cmp.i, i32 1, i32 %i
-; CHECKOLDHASH-SAME: !prof ![[BW:[0-9]+]]
-; CHECKOLDHASH: ![[BW]] = !{!"branch_weights", i32 6, i32 12}
   %retval.0.i = mul nsw i32 %mul.i, %i
   ret i32 %retval.0.i
 }

@kazutakahirata kazutakahirata merged commit b2b4ffb into llvm:main Jan 9, 2024
5 of 6 checks passed
@kazutakahirata kazutakahirata deleted the pr_cleanup_PGOOldCFGHashing branch January 9, 2024 05:06
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
It's been more than 3 years since -pgo-instr-old-cfg-hashing was
introduced by:

  commit 120e66b
  Author: Hiroshi Yamauchi <yamauchi@google.com>
  Date:   Tue Jul 28 10:09:49 2020 -0700

I don't think anyone really cares about the ability to use the old CFG
hashing at this point.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:transforms PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants