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

[UBSAN] Emit optimization remarks #88304

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

vitalybuka
Copy link
Collaborator

No description provided.

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 10, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Vitaly Buka (vitalybuka)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/IR/DiagnosticInfo.cpp (+5-1)
  • (modified) llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp (+29-3)
diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp
index 342c4cbbc39d65..31971b179fb4be 100644
--- a/llvm/lib/IR/DiagnosticInfo.cpp
+++ b/llvm/lib/IR/DiagnosticInfo.cpp
@@ -179,8 +179,12 @@ DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key,
   else if (isa<Constant>(V)) {
     raw_string_ostream OS(Val);
     V->printAsOperand(OS, /*PrintType=*/false);
-  } else if (auto *I = dyn_cast<Instruction>(V))
+  } else if (auto *I = dyn_cast<Instruction>(V)) {
     Val = I->getOpcodeName();
+  } else if (auto *MD = dyn_cast<MetadataAsValue>(V)) {
+    if (auto *S = dyn_cast<MDString>(MD->getMetadata()))
+      Val = S->getString();
+  }
 }
 
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, const Type *T)
diff --git a/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp b/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
index cdc8318f088c27..6c5a20227d0a36 100644
--- a/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
+++ b/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
@@ -10,11 +10,16 @@
 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/IR/Constant.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/Support/RandomNumberGenerator.h"
 #include <memory>
 #include <random>
@@ -35,8 +40,26 @@ static cl::opt<float>
 STATISTIC(NumChecksTotal, "Number of checks");
 STATISTIC(NumChecksRemoved, "Number of removed checks");
 
+static void emitRemark(IntrinsicInst *II, OptimizationRemarkEmitter &ORE,
+                       bool Removed) {
+  ore::NV Kind("Kind", II->getArgOperand(0));
+  ore::NV BB("Block", II->getParent()->getName());
+  if (Removed) {
+    ORE.emit([&]() {
+      return OptimizationRemark(DEBUG_TYPE, "Removed", II)
+             << "Removed check: Kind=" << Kind << " BB=" << BB;
+    });
+  } else {
+    ORE.emit([&]() {
+      return OptimizationRemarkMissed(DEBUG_TYPE, "Allowed", II)
+             << "Allowed check: Kind=" << Kind << " BB=" << BB;
+    });
+  }
+}
+
 static bool removeUbsanTraps(Function &F, const BlockFrequencyInfo &BFI,
-                             const ProfileSummaryInfo *PSI) {
+                             const ProfileSummaryInfo *PSI,
+                             OptimizationRemarkEmitter &ORE) {
   SmallVector<std::pair<IntrinsicInst *, bool>, 16> ReplaceWithValue;
   std::unique_ptr<RandomNumberGenerator> Rng;
 
@@ -75,6 +98,7 @@ static bool removeUbsanTraps(Function &F, const BlockFrequencyInfo &BFI,
         });
         if (ToRemove)
           ++NumChecksRemoved;
+        emitRemark(II, ORE, ToRemove);
         break;
       }
       default:
@@ -99,9 +123,11 @@ PreservedAnalyses LowerAllowCheckPass::run(Function &F,
   ProfileSummaryInfo *PSI =
       MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
   BlockFrequencyInfo &BFI = AM.getResult<BlockFrequencyAnalysis>(F);
+  OptimizationRemarkEmitter &ORE =
+      AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
 
-  return removeUbsanTraps(F, BFI, PSI) ? PreservedAnalyses::none()
-                                       : PreservedAnalyses::all();
+  return removeUbsanTraps(F, BFI, PSI, ORE) ? PreservedAnalyses::none()
+                                            : PreservedAnalyses::all();
 }
 
 bool LowerAllowCheckPass::IsRequested() {

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 10, 2024

@llvm/pr-subscribers-llvm-ir

Author: Vitaly Buka (vitalybuka)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/IR/DiagnosticInfo.cpp (+5-1)
  • (modified) llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp (+29-3)
diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp
index 342c4cbbc39d65..31971b179fb4be 100644
--- a/llvm/lib/IR/DiagnosticInfo.cpp
+++ b/llvm/lib/IR/DiagnosticInfo.cpp
@@ -179,8 +179,12 @@ DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key,
   else if (isa<Constant>(V)) {
     raw_string_ostream OS(Val);
     V->printAsOperand(OS, /*PrintType=*/false);
-  } else if (auto *I = dyn_cast<Instruction>(V))
+  } else if (auto *I = dyn_cast<Instruction>(V)) {
     Val = I->getOpcodeName();
+  } else if (auto *MD = dyn_cast<MetadataAsValue>(V)) {
+    if (auto *S = dyn_cast<MDString>(MD->getMetadata()))
+      Val = S->getString();
+  }
 }
 
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, const Type *T)
diff --git a/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp b/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
index cdc8318f088c27..6c5a20227d0a36 100644
--- a/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
+++ b/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
@@ -10,11 +10,16 @@
 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/IR/Constant.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/Support/RandomNumberGenerator.h"
 #include <memory>
 #include <random>
@@ -35,8 +40,26 @@ static cl::opt<float>
 STATISTIC(NumChecksTotal, "Number of checks");
 STATISTIC(NumChecksRemoved, "Number of removed checks");
 
+static void emitRemark(IntrinsicInst *II, OptimizationRemarkEmitter &ORE,
+                       bool Removed) {
+  ore::NV Kind("Kind", II->getArgOperand(0));
+  ore::NV BB("Block", II->getParent()->getName());
+  if (Removed) {
+    ORE.emit([&]() {
+      return OptimizationRemark(DEBUG_TYPE, "Removed", II)
+             << "Removed check: Kind=" << Kind << " BB=" << BB;
+    });
+  } else {
+    ORE.emit([&]() {
+      return OptimizationRemarkMissed(DEBUG_TYPE, "Allowed", II)
+             << "Allowed check: Kind=" << Kind << " BB=" << BB;
+    });
+  }
+}
+
 static bool removeUbsanTraps(Function &F, const BlockFrequencyInfo &BFI,
-                             const ProfileSummaryInfo *PSI) {
+                             const ProfileSummaryInfo *PSI,
+                             OptimizationRemarkEmitter &ORE) {
   SmallVector<std::pair<IntrinsicInst *, bool>, 16> ReplaceWithValue;
   std::unique_ptr<RandomNumberGenerator> Rng;
 
@@ -75,6 +98,7 @@ static bool removeUbsanTraps(Function &F, const BlockFrequencyInfo &BFI,
         });
         if (ToRemove)
           ++NumChecksRemoved;
+        emitRemark(II, ORE, ToRemove);
         break;
       }
       default:
@@ -99,9 +123,11 @@ PreservedAnalyses LowerAllowCheckPass::run(Function &F,
   ProfileSummaryInfo *PSI =
       MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
   BlockFrequencyInfo &BFI = AM.getResult<BlockFrequencyAnalysis>(F);
+  OptimizationRemarkEmitter &ORE =
+      AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
 
-  return removeUbsanTraps(F, BFI, PSI) ? PreservedAnalyses::none()
-                                       : PreservedAnalyses::all();
+  return removeUbsanTraps(F, BFI, PSI, ORE) ? PreservedAnalyses::none()
+                                            : PreservedAnalyses::all();
 }
 
 bool LowerAllowCheckPass::IsRequested() {

Created using spr 1.3.4
Copy link
Contributor

@jroelofs jroelofs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a test

@vitalybuka
Copy link
Collaborator Author

LGTM with a test

Oh, I didn't upload a test I have

Created using spr 1.3.4
@vitalybuka vitalybuka merged commit d927d18 into main Apr 10, 2024
3 of 4 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/ubsan-emit-optimization-remarks branch April 10, 2024 23:30
vitalybuka added a commit that referenced this pull request Apr 11, 2024
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.
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.

None yet

3 participants