[HWASAN] Emit optimization remarks

[vitalybuka]

Similar to #88304

SelHWAsan is optimization. We may want to diagnose compiler decisions.
Remarks is the tool for that https://llvm.org/docs/Remarks.html.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

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

Author: Vitaly Buka (vitalybuka)

Changes

Similar to #88304

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+35-15)
• (modified) llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll (+8-4)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 82f60f4feed454..3890aa8ca6ee60 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -21,6 +21,7 @@
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/DomTreeUpdater.h"
 #include "llvm/Analysis/GlobalsModRef.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/PostDominators.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/StackSafetyAnalysis.h"
@@ -1492,23 +1493,42 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
   return true;
 }
 
+static void emitRemark(const Function &F, OptimizationRemarkEmitter &ORE,
+                       bool Skip) {
+  if (Skip) {
+    ORE.emit([&]() {
+      return OptimizationRemark(DEBUG_TYPE, "Skip", &F)
+             << "Skipped: F=" << ore::NV("Function", &F);
+    });
+  } else {
+    ORE.emit([&]() {
+      return OptimizationRemarkMissed(DEBUG_TYPE, "Sanitize", &F)
+             << "Sanitized: F=" << ore::NV("Function", &F);
+    });
+  }
+}
+
 bool HWAddressSanitizer::selectiveInstrumentationShouldSkip(
     Function &F, FunctionAnalysisManager &FAM) const {
-  if (ClRandomSkipRate.getNumOccurrences()) {
-    std::bernoulli_distribution D(ClRandomSkipRate);
-    return !D(*Rng);
-  }
-  if (!ClHotPercentileCutoff.getNumOccurrences())
-    return false;
-  auto &MAMProxy = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
-  ProfileSummaryInfo *PSI =
-      MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
-  if (!PSI || !PSI->hasProfileSummary()) {
-    ++NumNoProfileSummaryFuncs;
-    return false;
-  }
-  return PSI->isFunctionHotInCallGraphNthPercentile(
-      ClHotPercentileCutoff, &F, FAM.getResult<BlockFrequencyAnalysis>(F));
+  bool Skip = [&]() {
+    if (ClRandomSkipRate.getNumOccurrences()) {
+      std::bernoulli_distribution D(ClRandomSkipRate);
+      return !D(*Rng);
+    }
+    if (!ClHotPercentileCutoff.getNumOccurrences())
+      return false;
+    auto &MAMProxy = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
+    ProfileSummaryInfo *PSI =
+        MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
+    if (!PSI || !PSI->hasProfileSummary()) {
+      ++NumNoProfileSummaryFuncs;
+      return false;
+    }
+    return PSI->isFunctionHotInCallGraphNthPercentile(
+        ClHotPercentileCutoff, &F, FAM.getResult<BlockFrequencyAnalysis>(F));
+  }();
+  emitRemark(F, FAM.getResult<OptimizationRemarkEmitterAnalysis>(F), Skip);
+  return Skip;
 }
 
 void HWAddressSanitizer::sanitizeFunction(Function &F,
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
index f2661a02da7a07..d5cb27d9b1ec1b 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
@@ -1,17 +1,21 @@
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-percentile-cutoff-hot=700000 | FileCheck %s --check-prefix=HOT70
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-percentile-cutoff-hot=990000 | FileCheck %s --check-prefix=HOT99
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-random-rate=1.0 | FileCheck %s --check-prefix=ALL
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-random-rate=0.0 | FileCheck %s --check-prefix=NONE
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -pass-remarks=hwasan -pass-remarks-missed=hwasan -S -hwasan-percentile-cutoff-hot=700000 2>&1 | FileCheck %s --check-prefix=HOT70
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -pass-remarks=hwasan -pass-remarks-missed=hwasan -S -hwasan-percentile-cutoff-hot=990000 2>&1 | FileCheck %s --check-prefix=HOT99
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -pass-remarks=hwasan -pass-remarks-missed=hwasan -S -hwasan-random-rate=1.0 2>&1 | FileCheck %s --check-prefix=ALL
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -pass-remarks=hwasan -pass-remarks-missed=hwasan -S -hwasan-random-rate=0.0 2>&1 | FileCheck %s --check-prefix=NONE
 
+; HOT70: remark: <unknown>:0:0: Sanitized: F=sanitized
 ; HOT70: @sanitized
 ; HOT70-NEXT: @__hwasan_tls
 
+; HOT99: remark: <unknown>:0:0: Skipped: F=sanitized
 ; HOT99: @sanitized
 ; HOT99-NEXT: %x = alloca i8, i64 4
 
+; ALL: remark: <unknown>:0:0: Sanitized: F=sanitize
 ; ALL: @sanitized
 ; ALL-NEXT: @__hwasan_tls
 
+; NONE: remark: <unknown>:0:0: Skipped: F=sanitized
 ; NONE: @sanitized
 ; NONE-NEXT: %x = alloca i8, i64 4
 

[fmayer]

Looks fine, could you put something in the description why we want this?

[vitalybuka]

Looks fine, could you put something in the description why we want this?

Done

[fmayer]

Why do we need this arg? We already have the F in the output either way.

Now the output looks like this

--- !Missed
Pass:            hwasan
Name:            Sanitize
Function:        test_retptr
Args:
  - String:          'Sanitized: F='
  - Function:        test_retptr
...

I suggest just making this OptimizationRemarkMissed(DEBUG_TYPE, "Sanitize", &F); Same above

[fmayer]

Also, wouldn't it be better to have the same name for the Skip and non skip-Cases, and just distinguish by the Miss / Pass