Skip to content

[HWASAN][UBSAN] Don't use default profile-summary-cutoff-hot #87691

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

Conversation

vitalybuka
Copy link
Collaborator

Default cutoff is not usefull here. Decision to
enable or not sanitizer causes more significant
performance impact, than a typical optimizations
which rely on profile-summary-cutoff-hot.

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Apr 4, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Vitaly Buka (vitalybuka)

Changes

Default cutoff is not usefull here. Decision to
enable or not sanitizer causes more significant
performance impact, than a typical optimizations
which rely on profile-summary-cutoff-hot.


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

4 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+4-10)
  • (modified) llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp (+3-10)
  • (modified) llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll (+1-1)
  • (modified) llvm/test/Transforms/RemoveTraps/remove-traps.ll (+1-1)
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 88e84ed7be8e6a..8562e2efd33e10 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -187,10 +187,8 @@ static cl::opt<bool>
                               cl::desc("Use selective instrumentation"),
                               cl::Hidden, cl::init(false));
 
-static cl::opt<int> ClHotPercentileCutoff(
-    "hwasan-percentile-cutoff-hot", cl::init(0),
-    cl::desc("Alternative hot percentile cuttoff."
-             "By default `-profile-summary-cutoff-hot` is used."));
+static cl::opt<int> ClHotPercentileCutoff("hwasan-percentile-cutoff-hot",
+                                          cl::desc("Hot percentile cuttoff."));
 
 static cl::opt<float>
     ClRandomSkipRate("hwasan-random-skip-rate", cl::init(0),
@@ -1512,12 +1510,8 @@ bool HWAddressSanitizer::selectiveInstrumentationShouldSkip(
     ++NumNoProfileSummaryFuncs;
     return false;
   }
-  auto &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
-  return (
-      (ClHotPercentileCutoff.getNumOccurrences() && ClHotPercentileCutoff >= 0)
-          ? PSI->isFunctionHotInCallGraphNthPercentile(ClHotPercentileCutoff,
-                                                       &F, BFI)
-          : PSI->isFunctionHotInCallGraph(&F, BFI));
+  return PSI->isFunctionHotInCallGraphNthPercentile(
+      ClHotPercentileCutoff, &F, FAM.getResult<BlockFrequencyAnalysis>(F));
 }
 
 void HWAddressSanitizer::sanitizeFunction(Function &F,
diff --git a/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp b/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp
index d87f7482a21d25..6bcbccda031cec 100644
--- a/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp
+++ b/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp
@@ -22,10 +22,8 @@ using namespace llvm;
 
 #define DEBUG_TYPE "remove-traps"
 
-static cl::opt<int> HotPercentileCutoff(
-    "remove-traps-percentile-cutoff-hot", cl::init(0),
-    cl::desc("Alternative hot percentile cuttoff. By default "
-             "`-profile-summary-cutoff-hot` is used."));
+static cl::opt<int> HotPercentileCutoff("remove-traps-percentile-cutoff-hot",
+                                        cl::desc("Hot percentile cuttoff."));
 
 static cl::opt<float>
     RandomRate("remove-traps-random-rate", cl::init(0.0),
@@ -64,12 +62,7 @@ static bool removeUbsanTraps(Function &F, const BlockFrequencyInfo &BFI,
           uint64_t Count = 0;
           for (const auto *PR : predecessors(&BB))
             Count += BFI.getBlockProfileCount(PR).value_or(0);
-
-          IsHot =
-              HotPercentileCutoff.getNumOccurrences()
-                  ? (HotPercentileCutoff > 0 &&
-                     PSI->isHotCountNthPercentile(HotPercentileCutoff, Count))
-                  : PSI->isHotCount(Count);
+          IsHot = PSI->isHotCountNthPercentile(HotPercentileCutoff, Count);
         }
 
         if (ShouldRemove(IsHot)) {
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
index e568f5b88dc145..da9bff8c048cc6 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
@@ -1,7 +1,7 @@
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
 ; RUN:    -hwasan-percentile-cutoff-hot=700000 | FileCheck %s --check-prefix=HOT70
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
-; RUN:                                 | FileCheck %s --check-prefix=HOT99
+; RUN:    -hwasan-percentile-cutoff-hot=990000 | FileCheck %s --check-prefix=HOT99
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
 ; RUN:    -hwasan-random-skip-rate=0.0 | FileCheck %s --check-prefix=RANDOM0
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
diff --git a/llvm/test/Transforms/RemoveTraps/remove-traps.ll b/llvm/test/Transforms/RemoveTraps/remove-traps.ll
index e3cca83884a8e1..4853149f955b09 100644
--- a/llvm/test/Transforms/RemoveTraps/remove-traps.ll
+++ b/llvm/test/Transforms/RemoveTraps/remove-traps.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
 ; RUN: opt < %s -passes='function(remove-traps)' -S | FileCheck %s --check-prefixes=NOPROFILE
 ; RUN: opt < %s -passes='function(remove-traps)' -remove-traps-random-rate=1 -S | FileCheck %s --check-prefixes=ALL
-; RUN: opt < %s -passes='require<profile-summary>,function(remove-traps)' -S | FileCheck %s --check-prefixes=HOT99
+; RUN: opt < %s -passes='require<profile-summary>,function(remove-traps)' -remove-traps-percentile-cutoff-hot=990000 -S | FileCheck %s --check-prefixes=HOT99
 ; RUN: opt < %s -passes='require<profile-summary>,function(remove-traps)' -remove-traps-percentile-cutoff-hot=700000 -S | FileCheck %s --check-prefixes=HOT70
 
 target triple = "x86_64-pc-linux-gnu"

@llvmbot
Copy link
Member

llvmbot commented Apr 4, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

Changes

Default cutoff is not usefull here. Decision to
enable or not sanitizer causes more significant
performance impact, than a typical optimizations
which rely on profile-summary-cutoff-hot.


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

4 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+4-10)
  • (modified) llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp (+3-10)
  • (modified) llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll (+1-1)
  • (modified) llvm/test/Transforms/RemoveTraps/remove-traps.ll (+1-1)
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 88e84ed7be8e6a..8562e2efd33e10 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -187,10 +187,8 @@ static cl::opt<bool>
                               cl::desc("Use selective instrumentation"),
                               cl::Hidden, cl::init(false));
 
-static cl::opt<int> ClHotPercentileCutoff(
-    "hwasan-percentile-cutoff-hot", cl::init(0),
-    cl::desc("Alternative hot percentile cuttoff."
-             "By default `-profile-summary-cutoff-hot` is used."));
+static cl::opt<int> ClHotPercentileCutoff("hwasan-percentile-cutoff-hot",
+                                          cl::desc("Hot percentile cuttoff."));
 
 static cl::opt<float>
     ClRandomSkipRate("hwasan-random-skip-rate", cl::init(0),
@@ -1512,12 +1510,8 @@ bool HWAddressSanitizer::selectiveInstrumentationShouldSkip(
     ++NumNoProfileSummaryFuncs;
     return false;
   }
-  auto &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
-  return (
-      (ClHotPercentileCutoff.getNumOccurrences() && ClHotPercentileCutoff >= 0)
-          ? PSI->isFunctionHotInCallGraphNthPercentile(ClHotPercentileCutoff,
-                                                       &F, BFI)
-          : PSI->isFunctionHotInCallGraph(&F, BFI));
+  return PSI->isFunctionHotInCallGraphNthPercentile(
+      ClHotPercentileCutoff, &F, FAM.getResult<BlockFrequencyAnalysis>(F));
 }
 
 void HWAddressSanitizer::sanitizeFunction(Function &F,
diff --git a/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp b/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp
index d87f7482a21d25..6bcbccda031cec 100644
--- a/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp
+++ b/llvm/lib/Transforms/Instrumentation/RemoveTrapsPass.cpp
@@ -22,10 +22,8 @@ using namespace llvm;
 
 #define DEBUG_TYPE "remove-traps"
 
-static cl::opt<int> HotPercentileCutoff(
-    "remove-traps-percentile-cutoff-hot", cl::init(0),
-    cl::desc("Alternative hot percentile cuttoff. By default "
-             "`-profile-summary-cutoff-hot` is used."));
+static cl::opt<int> HotPercentileCutoff("remove-traps-percentile-cutoff-hot",
+                                        cl::desc("Hot percentile cuttoff."));
 
 static cl::opt<float>
     RandomRate("remove-traps-random-rate", cl::init(0.0),
@@ -64,12 +62,7 @@ static bool removeUbsanTraps(Function &F, const BlockFrequencyInfo &BFI,
           uint64_t Count = 0;
           for (const auto *PR : predecessors(&BB))
             Count += BFI.getBlockProfileCount(PR).value_or(0);
-
-          IsHot =
-              HotPercentileCutoff.getNumOccurrences()
-                  ? (HotPercentileCutoff > 0 &&
-                     PSI->isHotCountNthPercentile(HotPercentileCutoff, Count))
-                  : PSI->isHotCount(Count);
+          IsHot = PSI->isHotCountNthPercentile(HotPercentileCutoff, Count);
         }
 
         if (ShouldRemove(IsHot)) {
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
index e568f5b88dc145..da9bff8c048cc6 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
@@ -1,7 +1,7 @@
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
 ; RUN:    -hwasan-percentile-cutoff-hot=700000 | FileCheck %s --check-prefix=HOT70
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
-; RUN:                                 | FileCheck %s --check-prefix=HOT99
+; RUN:    -hwasan-percentile-cutoff-hot=990000 | FileCheck %s --check-prefix=HOT99
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
 ; RUN:    -hwasan-random-skip-rate=0.0 | FileCheck %s --check-prefix=RANDOM0
 ; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-selective-instrumentation=1 \
diff --git a/llvm/test/Transforms/RemoveTraps/remove-traps.ll b/llvm/test/Transforms/RemoveTraps/remove-traps.ll
index e3cca83884a8e1..4853149f955b09 100644
--- a/llvm/test/Transforms/RemoveTraps/remove-traps.ll
+++ b/llvm/test/Transforms/RemoveTraps/remove-traps.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
 ; RUN: opt < %s -passes='function(remove-traps)' -S | FileCheck %s --check-prefixes=NOPROFILE
 ; RUN: opt < %s -passes='function(remove-traps)' -remove-traps-random-rate=1 -S | FileCheck %s --check-prefixes=ALL
-; RUN: opt < %s -passes='require<profile-summary>,function(remove-traps)' -S | FileCheck %s --check-prefixes=HOT99
+; RUN: opt < %s -passes='require<profile-summary>,function(remove-traps)' -remove-traps-percentile-cutoff-hot=990000 -S | FileCheck %s --check-prefixes=HOT99
 ; RUN: opt < %s -passes='require<profile-summary>,function(remove-traps)' -remove-traps-percentile-cutoff-hot=700000 -S | FileCheck %s --check-prefixes=HOT70
 
 target triple = "x86_64-pc-linux-gnu"

Created using spr 1.3.4
@vitalybuka vitalybuka requested a review from fmayer April 4, 2024 21:20
@vitalybuka vitalybuka merged commit 03f5472 into main Apr 4, 2024
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/hwasanubsan-dont-use-default-profile-summary-cutoff-hot branch April 4, 2024 21:25
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.

3 participants