Skip to content

Conversation

@thurstond
Copy link
Contributor

The meaning of ClRandomSkipRate was inverted in
#88070 but the variable name was not changed. This patch fixes it to avoid confusion.

Additionally, it elaborates the flag description to mention the interaction between the random keep rate and hotness cutoff.

The meaning of ClRandomSkipRate was inverted in
llvm#88070 but the variable name
was not changed. This patch fixes it to avoid confusion.

Additionally, it elaborates the flag description to mention the
interaction between the random keep rate and hotness cutoff.
@llvmbot
Copy link
Member

llvmbot commented Feb 13, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Thurston Dang (thurstond)

Changes

The meaning of ClRandomSkipRate was inverted in
#88070 but the variable name was not changed. This patch fixes it to avoid confusion.

Additionally, it elaborates the flag description to mention the interaction between the random keep rate and hotness cutoff.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+7-5)
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 645c102752692..5a184cc066aab 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -195,9 +195,11 @@ static cl::opt<int> ClHotPercentileCutoff("hwasan-percentile-cutoff-hot",
                                           cl::desc("Hot percentile cutoff."));
 
 static cl::opt<float>
-    ClRandomSkipRate("hwasan-random-rate",
+    ClRandomKeepRate("hwasan-random-rate",
                      cl::desc("Probability value in the range [0.0, 1.0] "
-                              "to keep instrumentation of a function."));
+                              "to keep instrumentation of a function. "
+                              "Note: instrumentation can be skipped randomly "
+                              "OR because of the hot percentile cutoff."));
 
 STATISTIC(NumTotalFuncs, "Number of total funcs");
 STATISTIC(NumInstrumentedFuncs, "Number of instrumented funcs");
@@ -301,7 +303,7 @@ class HWAddressSanitizer {
       : M(M), SSI(SSI) {
     this->Recover = optOr(ClRecover, Recover);
     this->CompileKernel = optOr(ClEnableKhwasan, CompileKernel);
-    this->Rng = ClRandomSkipRate.getNumOccurrences() ? M.createRNG(DEBUG_TYPE)
+    this->Rng = ClRandomKeepRate.getNumOccurrences() ? M.createRNG(DEBUG_TYPE)
                                                      : nullptr;
 
     initializeModule();
@@ -1599,9 +1601,9 @@ bool HWAddressSanitizer::selectiveInstrumentationShouldSkip(
   };
 
   auto SkipRandom = [&]() {
-    if (!ClRandomSkipRate.getNumOccurrences())
+    if (!ClRandomKeepRate.getNumOccurrences())
       return false;
-    std::bernoulli_distribution D(ClRandomSkipRate);
+    std::bernoulli_distribution D(ClRandomKeepRate);
     return !D(*Rng);
   };
 

@llvmbot
Copy link
Member

llvmbot commented Feb 13, 2025

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

Author: Thurston Dang (thurstond)

Changes

The meaning of ClRandomSkipRate was inverted in
#88070 but the variable name was not changed. This patch fixes it to avoid confusion.

Additionally, it elaborates the flag description to mention the interaction between the random keep rate and hotness cutoff.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+7-5)
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 645c102752692..5a184cc066aab 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -195,9 +195,11 @@ static cl::opt<int> ClHotPercentileCutoff("hwasan-percentile-cutoff-hot",
                                           cl::desc("Hot percentile cutoff."));
 
 static cl::opt<float>
-    ClRandomSkipRate("hwasan-random-rate",
+    ClRandomKeepRate("hwasan-random-rate",
                      cl::desc("Probability value in the range [0.0, 1.0] "
-                              "to keep instrumentation of a function."));
+                              "to keep instrumentation of a function. "
+                              "Note: instrumentation can be skipped randomly "
+                              "OR because of the hot percentile cutoff."));
 
 STATISTIC(NumTotalFuncs, "Number of total funcs");
 STATISTIC(NumInstrumentedFuncs, "Number of instrumented funcs");
@@ -301,7 +303,7 @@ class HWAddressSanitizer {
       : M(M), SSI(SSI) {
     this->Recover = optOr(ClRecover, Recover);
     this->CompileKernel = optOr(ClEnableKhwasan, CompileKernel);
-    this->Rng = ClRandomSkipRate.getNumOccurrences() ? M.createRNG(DEBUG_TYPE)
+    this->Rng = ClRandomKeepRate.getNumOccurrences() ? M.createRNG(DEBUG_TYPE)
                                                      : nullptr;
 
     initializeModule();
@@ -1599,9 +1601,9 @@ bool HWAddressSanitizer::selectiveInstrumentationShouldSkip(
   };
 
   auto SkipRandom = [&]() {
-    if (!ClRandomSkipRate.getNumOccurrences())
+    if (!ClRandomKeepRate.getNumOccurrences())
       return false;
-    std::bernoulli_distribution D(ClRandomSkipRate);
+    std::bernoulli_distribution D(ClRandomKeepRate);
     return !D(*Rng);
   };
 

@thurstond thurstond merged commit df07121 into llvm:main Feb 13, 2025
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 13, 2025

LLVM Buildbot has detected a new failure on builder openmp-s390x-linux running on systemz-1 while building llvm at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/7968

Here is the relevant piece of the build log for the reference
Step 6 (test-openmp) failure: test (failure)
******************** TEST 'libomp :: tasking/issue-94260-2.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp   -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic && /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

********************


flovent pushed a commit to flovent/llvm-project that referenced this pull request Feb 13, 2025
The meaning of ClRandomSkipRate was inverted in
llvm#88070 but the variable name
was not changed. This patch fixes it to avoid confusion.

Additionally, it elaborates the flag description to mention the
interaction between the random keep rate and hotness cutoff.
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
The meaning of ClRandomSkipRate was inverted in
llvm#88070 but the variable name
was not changed. This patch fixes it to avoid confusion.

Additionally, it elaborates the flag description to mention the
interaction between the random keep rate and hotness cutoff.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
The meaning of ClRandomSkipRate was inverted in
llvm#88070 but the variable name
was not changed. This patch fixes it to avoid confusion.

Additionally, it elaborates the flag description to mention the
interaction between the random keep rate and hotness cutoff.
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.

5 participants