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

[NFCI][sanitizers][metadata] Exctract create{Unlikely,Likely}BranchWeights #89464

Conversation

vitalybuka
Copy link
Collaborator

We have a lot of repeated code with random constants.
Particular values are not important, the one just needs to be
bigger then another.

UR_NONTAKEN_WEIGHT is selected as it's the most common one.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-ir

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

Author: Vitaly Buka (vitalybuka)

Changes

We have a lot of repeated code with random constants.
Particular values are not important, the one just needs to be
bigger then another.

UR_NONTAKEN_WEIGHT is selected as it's the most common one.


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

14 Files Affected:

  • (modified) llvm/include/llvm/IR/MDBuilder.h (+8)
  • (modified) llvm/lib/IR/MDBuilder.cpp (+10)
  • (modified) llvm/lib/Transforms/IPO/CrossDSOCFI.cpp (+1-2)
  • (modified) llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp (+1-2)
  • (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+2-2)
  • (modified) llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp (+2-2)
  • (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+4-4)
  • (modified) llvm/lib/Transforms/Instrumentation/KCFI.cpp (+1-2)
  • (modified) llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp (+2-2)
  • (modified) llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp (+2-2)
  • (modified) llvm/test/Instrumentation/AddressSanitizer/asan-funclet.ll (+1-1)
  • (modified) llvm/test/Instrumentation/AddressSanitizer/basic.ll (+1-1)
  • (modified) llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll (+1-1)
  • (modified) llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll (+2-2)
diff --git a/llvm/include/llvm/IR/MDBuilder.h b/llvm/include/llvm/IR/MDBuilder.h
index 39165453de16b0..3265589b7c8dfa 100644
--- a/llvm/include/llvm/IR/MDBuilder.h
+++ b/llvm/include/llvm/IR/MDBuilder.h
@@ -61,6 +61,14 @@ class MDBuilder {
   /// Return metadata containing two branch weights.
   MDNode *createBranchWeights(uint32_t TrueWeight, uint32_t FalseWeight);
 
+  /// Return metadata containing two branch weights, with significant bias
+  /// towards `true` destination.
+  MDNode *createLikelyBranchWeights();
+
+  /// Return metadata containing two branch weights, with significant bias
+  /// towards `false` destination.
+  MDNode *createUnlikelyBranchWeights();
+
   /// Return metadata containing a number of branch weights.
   MDNode *createBranchWeights(ArrayRef<uint32_t> Weights);
 
diff --git a/llvm/lib/IR/MDBuilder.cpp b/llvm/lib/IR/MDBuilder.cpp
index 2490b3012bdc2b..0bf41d7cc7c2cd 100644
--- a/llvm/lib/IR/MDBuilder.cpp
+++ b/llvm/lib/IR/MDBuilder.cpp
@@ -39,6 +39,16 @@ MDNode *MDBuilder::createBranchWeights(uint32_t TrueWeight,
   return createBranchWeights({TrueWeight, FalseWeight});
 }
 
+MDNode *MDBuilder::createLikelyBranchWeights() {
+  // Value chosen to match UR_NONTAKEN_WEIGHT, see BranchProbabilityInfo.cpp
+  return createBranchWeights((1U << 20) - 1, 1);
+}
+
+MDNode *MDBuilder::createUnlikelyBranchWeights() {
+  // Value chosen to match UR_NONTAKEN_WEIGHT, see BranchProbabilityInfo.cpp
+  return createBranchWeights(1, (1U << 20) - 1);
+}
+
 MDNode *MDBuilder::createBranchWeights(ArrayRef<uint32_t> Weights) {
   assert(Weights.size() >= 1 && "Need at least one branch weights!");
 
diff --git a/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp b/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp
index 5cc8258a495a6e..91d445dfc4c734 100644
--- a/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp
+++ b/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp
@@ -139,8 +139,7 @@ void CrossDSOCFI::buildCFICheck(Module &M) {
 }
 
 bool CrossDSOCFI::runOnModule(Module &M) {
-  VeryLikelyWeights =
-    MDBuilder(M.getContext()).createBranchWeights((1U << 20) - 1, 1);
+  VeryLikelyWeights = MDBuilder(M.getContext()).createLikelyBranchWeights();
   if (M.getModuleFlag("Cross-DSO CFI") == nullptr)
     return false;
   buildCFICheck(M);
diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index cf13c91a867704..e7a188e9431db5 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -1196,8 +1196,7 @@ void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
       // function pointer to the devirtualized target. In case of a mismatch,
       // fall back to indirect call.
       if (DevirtCheckMode == WPDCheckMode::Fallback) {
-        MDNode *Weights =
-            MDBuilder(M.getContext()).createBranchWeights((1U << 20) - 1, 1);
+        MDNode *Weights = MDBuilder(M.getContext()).createLikelyBranchWeights();
         // Version the indirect call site. If the called value is equal to the
         // given callee, 'NewInst' will be executed, otherwise the original call
         // site will be executed.
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 26fedbfd65dd41..9cc978dc6c16ef 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1817,7 +1817,7 @@ Instruction *AddressSanitizer::genAMDGPUReportBlock(IRBuilder<> &IRB,
 
   auto *Trm =
       SplitBlockAndInsertIfThen(ReportCond, &*IRB.GetInsertPoint(), false,
-                                MDBuilder(*C).createBranchWeights(1, 100000));
+                                MDBuilder(*C).createUnlikelyBranchWeights());
   Trm->getParent()->setName("asan.report");
 
   if (Recover)
@@ -1894,7 +1894,7 @@ void AddressSanitizer::instrumentAddress(Instruction *OrigIns,
     // We use branch weights for the slow path check, to indicate that the slow
     // path is rarely taken. This seems to be the case for SPEC benchmarks.
     Instruction *CheckTerm = SplitBlockAndInsertIfThen(
-        Cmp, InsertBefore, false, MDBuilder(*C).createBranchWeights(1, 100000));
+        Cmp, InsertBefore, false, MDBuilder(*C).createUnlikelyBranchWeights());
     assert(cast<BranchInst>(CheckTerm)->isUnconditional());
     BasicBlock *NextBB = CheckTerm->getSuccessor(0);
     IRB.SetInsertPoint(CheckTerm);
diff --git a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
index 9a2ec7618c1a49..cdd891fc89483d 100644
--- a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -1229,8 +1229,8 @@ bool DataFlowSanitizer::initializeModule(Module &M) {
       FunctionType::get(Type::getVoidTy(*Ctx), DFSanMemTransferCallbackArgs,
                         /*isVarArg=*/false);
 
-  ColdCallWeights = MDBuilder(*Ctx).createBranchWeights(1, 1000);
-  OriginStoreWeights = MDBuilder(*Ctx).createBranchWeights(1, 1000);
+  ColdCallWeights = MDBuilder(*Ctx).createUnlikelyBranchWeights();
+  OriginStoreWeights = MDBuilder(*Ctx).createUnlikelyBranchWeights();
   return true;
 }
 
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 3890aa8ca6ee60..865722d0259812 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -911,7 +911,7 @@ HWAddressSanitizer::insertShadowTagCheck(Value *Ptr, Instruction *InsertBefore,
 
   R.TagMismatchTerm = SplitBlockAndInsertIfThen(
       TagMismatch, InsertBefore, false,
-      MDBuilder(*C).createBranchWeights(1, 100000), &DTU, LI);
+      MDBuilder(*C).createUnlikelyBranchWeights(), &DTU, LI);
 
   return R;
 }
@@ -952,7 +952,7 @@ void HWAddressSanitizer::instrumentMemAccessInline(Value *Ptr, bool IsWrite,
       IRB.CreateICmpUGT(TCI.MemTag, ConstantInt::get(Int8Ty, 15));
   Instruction *CheckFailTerm = SplitBlockAndInsertIfThen(
       OutOfShortGranuleTagRange, TCI.TagMismatchTerm, !Recover,
-      MDBuilder(*C).createBranchWeights(1, 100000), &DTU, LI);
+      MDBuilder(*C).createUnlikelyBranchWeights(), &DTU, LI);
 
   IRB.SetInsertPoint(TCI.TagMismatchTerm);
   Value *PtrLowBits = IRB.CreateTrunc(IRB.CreateAnd(TCI.PtrLong, 15), Int8Ty);
@@ -960,7 +960,7 @@ void HWAddressSanitizer::instrumentMemAccessInline(Value *Ptr, bool IsWrite,
       PtrLowBits, ConstantInt::get(Int8Ty, (1 << AccessSizeIndex) - 1));
   Value *PtrLowBitsOOB = IRB.CreateICmpUGE(PtrLowBits, TCI.MemTag);
   SplitBlockAndInsertIfThen(PtrLowBitsOOB, TCI.TagMismatchTerm, false,
-                            MDBuilder(*C).createBranchWeights(1, 100000), &DTU,
+                            MDBuilder(*C).createUnlikelyBranchWeights(), &DTU,
                             LI, CheckFailTerm->getParent());
 
   IRB.SetInsertPoint(TCI.TagMismatchTerm);
@@ -969,7 +969,7 @@ void HWAddressSanitizer::instrumentMemAccessInline(Value *Ptr, bool IsWrite,
   Value *InlineTag = IRB.CreateLoad(Int8Ty, InlineTagAddr);
   Value *InlineTagMismatch = IRB.CreateICmpNE(TCI.PtrTag, InlineTag);
   SplitBlockAndInsertIfThen(InlineTagMismatch, TCI.TagMismatchTerm, false,
-                            MDBuilder(*C).createBranchWeights(1, 100000), &DTU,
+                            MDBuilder(*C).createUnlikelyBranchWeights(), &DTU,
                             LI, CheckFailTerm->getParent());
 
   IRB.SetInsertPoint(CheckFailTerm);
diff --git a/llvm/lib/Transforms/Instrumentation/KCFI.cpp b/llvm/lib/Transforms/Instrumentation/KCFI.cpp
index b22e7f7fc0bee0..28dc1c02b661ac 100644
--- a/llvm/lib/Transforms/Instrumentation/KCFI.cpp
+++ b/llvm/lib/Transforms/Instrumentation/KCFI.cpp
@@ -71,8 +71,7 @@ PreservedAnalyses KCFIPass::run(Function &F, FunctionAnalysisManager &AM) {
                            "compatible with -fsanitize=kcfi on this target"));
 
   IntegerType *Int32Ty = Type::getInt32Ty(Ctx);
-  MDNode *VeryUnlikelyWeights =
-      MDBuilder(Ctx).createBranchWeights(1, (1U << 20) - 1);
+  MDNode *VeryUnlikelyWeights = MDBuilder(Ctx).createUnlikelyBranchWeights();
   Triple T(M.getTargetTriple());
 
   for (CallInst *CI : KCFICalls) {
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index ee3531bbd68df3..1d7d0356ea6956 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1040,8 +1040,8 @@ void MemorySanitizer::initializeModule(Module &M) {
   OriginTy = IRB.getInt32Ty();
   PtrTy = IRB.getPtrTy();
 
-  ColdCallWeights = MDBuilder(*C).createBranchWeights(1, 1000);
-  OriginStoreWeights = MDBuilder(*C).createBranchWeights(1, 1000);
+  ColdCallWeights = MDBuilder(*C).createUnlikelyBranchWeights();
+  OriginStoreWeights = MDBuilder(*C).createUnlikelyBranchWeights();
 
   if (!CompileKernel) {
     if (TrackOrigins)
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index 123500ef2fef65..b7d4da07da8c4d 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -982,7 +982,7 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
     auto Load = IRB.CreateLoad(Int1Ty, FlagPtr);
     auto ThenTerm = SplitBlockAndInsertIfThen(
         IRB.CreateIsNull(Load), &*IP, false,
-        MDBuilder(IRB.getContext()).createBranchWeights(1, 100000));
+        MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
     IRBuilder<> ThenIRB(ThenTerm);
     auto Store = ThenIRB.CreateStore(ConstantInt::getTrue(Int1Ty), FlagPtr);
     Load->setNoSanitizeMetadata();
@@ -1001,7 +1001,7 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
     auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
     auto ThenTerm = SplitBlockAndInsertIfThen(
         IsStackLower, &*IP, false,
-        MDBuilder(IRB.getContext()).createBranchWeights(1, 100000));
+        MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
     IRBuilder<> ThenIRB(ThenTerm);
     auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
     LowestStack->setNoSanitizeMetadata();
diff --git a/llvm/test/Instrumentation/AddressSanitizer/asan-funclet.ll b/llvm/test/Instrumentation/AddressSanitizer/asan-funclet.ll
index 6d40e2b7eaf9c5..62c0a24c60d77f 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/asan-funclet.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/asan-funclet.ll
@@ -580,5 +580,5 @@ ehcleanup:                                        ; preds = %entry
   cleanupret from %0 unwind to caller
 }
 ;.
-; CHECK-INLINE: [[PROF0]] = !{!"branch_weights", i32 1, i32 100000}
+; CHECK-INLINE: [[PROF0]] = !{!"branch_weights", i32 1, i32 1048575}
 ;.
diff --git a/llvm/test/Instrumentation/AddressSanitizer/basic.ll b/llvm/test/Instrumentation/AddressSanitizer/basic.ll
index 2064db5a0918fc..1aef8c03c9d3e5 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/basic.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/basic.ll
@@ -218,4 +218,4 @@ define void @test_swifterror_3() sanitize_address {
 ; CHECK: attributes #[[#ATTR]] = { nounwind }
 
 ; PROF
-; CHECK: ![[PROF]] = !{!"branch_weights", i32 1, i32 100000}
+; CHECK: ![[PROF]] = !{!"branch_weights", i32 1, i32 1048575}
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll b/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
index 184de7e7bce4f2..a6d7ae67d8453c 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
@@ -34,5 +34,5 @@ entry:
 ; CHECK: attributes #[[ATTR0:[0-9]+]] = { nounwind }
 ;.
 ; CHECK: [[META0]] = !{}
-; CHECK: [[PROF1]] = !{!"branch_weights", i32 1, i32 100000}
+; CHECK: [[PROF1]] = !{!"branch_weights", i32 1, i32 1048575}
 ;.
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll b/llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
index ee0caa7020fdf9..00547afd1b9ef2 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -90,8 +90,8 @@ define weak_odr hidden ptr @_ZTW21__sancov_lowest_stack() {
 ; L3: attributes #[[ATTR2]] = { nomerge }
 ;.
 ; L1: [[META0]] = !{}
-; L1: [[PROF1]] = !{!"branch_weights", i32 1, i32 100000}
+; L1: [[PROF1]] = !{!"branch_weights", i32 1, i32 1048575}
 ;.
 ; L3: [[META0]] = !{}
-; L3: [[PROF1]] = !{!"branch_weights", i32 1, i32 100000}
+; L3: [[PROF1]] = !{!"branch_weights", i32 1, i32 1048575}
 ;.

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.nfcisanitizersmetadata-exctract-createunlikelylikelybranchweights to main April 20, 2024 00:02
@vitalybuka vitalybuka merged commit c60aa43 into main Apr 20, 2024
4 of 5 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcisanitizersmetadata-exctract-createunlikelylikelybranchweights branch April 20, 2024 00:03
aniplcc pushed a commit to aniplcc/llvm-project that referenced this pull request Apr 21, 2024
…ights (llvm#89464)

We have a lot of repeated code with random constants.
Particular values are not important, the one just needs to be
bigger then another.

UR_NONTAKEN_WEIGHT is selected as it's the most common one.
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