[llvm] [MC/DC] Rework tvbitmap.update to get rid of the inlined function (PR #110792)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 1 22:29:33 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: NAKAMURA Takumi (chapuni)
<details>
<summary>Changes</summary>
Per the discussion in #<!-- -->102542, it is safe to insert BBs under `lowerIntrinsics()` since #<!-- -->69535 has made tolerant of modifying BBs.
So, I can get rid of using the inlined function `rmw_or`, introduced in #<!-- -->96040.
---
Full diff: https://github.com/llvm/llvm-project/pull/110792.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp (+33-77)
- (modified) llvm/test/Instrumentation/InstrProfiling/mcdc.ll (+12-16)
``````````diff
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index fe882164656dfb..1c95a4606ecc56 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -267,10 +267,6 @@ class InstrLowerer final {
GlobalVariable *NamesVar = nullptr;
size_t NamesSize = 0;
- /// The instance of [[alwaysinline]] rmw_or(ptr, i8).
- /// This is name-insensitive.
- Function *RMWOrFunc = nullptr;
-
// vector of counter load/store pairs to be register promoted.
std::vector<LoadStorePair> PromotionCandidates;
@@ -337,14 +333,6 @@ class InstrLowerer final {
StringRef Name,
GlobalValue::LinkageTypes Linkage);
- /// Create [[alwaysinline]] rmw_or(ptr, i8).
- /// This doesn't update `RMWOrFunc`.
- Function *createRMWOrFunc();
-
- /// Get the call to `rmw_or`.
- /// Create the instance if it is unknown.
- CallInst *getRMWOrCall(Value *Addr, Value *Val);
-
/// Compute the address of the test vector bitmap that this profiling
/// instruction acts on.
Value *getBitmapAddress(InstrProfMCDCTVBitmapUpdate *I);
@@ -1137,68 +1125,6 @@ Value *InstrLowerer::getCounterAddress(InstrProfCntrInstBase *I) {
return Builder.CreateIntToPtr(Add, Addr->getType());
}
-/// Create `void [[alwaysinline]] rmw_or(uint8_t *ArgAddr, uint8_t ArgVal)`
-/// "Basic" sequence is `*ArgAddr |= ArgVal`
-Function *InstrLowerer::createRMWOrFunc() {
- auto &Ctx = M.getContext();
- auto *Int8Ty = Type::getInt8Ty(Ctx);
- Function *Fn = Function::Create(
- FunctionType::get(Type::getVoidTy(Ctx),
- {PointerType::getUnqual(Ctx), Int8Ty}, false),
- Function::LinkageTypes::PrivateLinkage, "rmw_or", M);
- Fn->addFnAttr(Attribute::AlwaysInline);
- auto *ArgAddr = Fn->getArg(0);
- auto *ArgVal = Fn->getArg(1);
- IRBuilder<> Builder(BasicBlock::Create(Ctx, "", Fn));
-
- // Load profile bitmap byte.
- // %mcdc.bits = load i8, ptr %4, align 1
- auto *Bitmap = Builder.CreateLoad(Int8Ty, ArgAddr, "mcdc.bits");
-
- if (Options.Atomic || AtomicCounterUpdateAll) {
- // If ((Bitmap & Val) != Val), then execute atomic (Bitmap |= Val).
- // Note, just-loaded Bitmap might not be up-to-date. Use it just for
- // early testing.
- auto *Masked = Builder.CreateAnd(Bitmap, ArgVal);
- auto *ShouldStore = Builder.CreateICmpNE(Masked, ArgVal);
- auto *ThenTerm = BasicBlock::Create(Ctx, "", Fn);
- auto *ElseTerm = BasicBlock::Create(Ctx, "", Fn);
- // Assume updating will be rare.
- auto *Unlikely = MDBuilder(Ctx).createUnlikelyBranchWeights();
- Builder.CreateCondBr(ShouldStore, ThenTerm, ElseTerm, Unlikely);
-
- IRBuilder<> ThenBuilder(ThenTerm);
- ThenBuilder.CreateAtomicRMW(AtomicRMWInst::Or, ArgAddr, ArgVal,
- MaybeAlign(), AtomicOrdering::Monotonic);
- ThenBuilder.CreateRetVoid();
-
- IRBuilder<> ElseBuilder(ElseTerm);
- ElseBuilder.CreateRetVoid();
-
- return Fn;
- }
-
- // Perform logical OR of profile bitmap byte and shifted bit offset.
- // %8 = or i8 %mcdc.bits, %7
- auto *Result = Builder.CreateOr(Bitmap, ArgVal);
-
- // Store the updated profile bitmap byte.
- // store i8 %8, ptr %3, align 1
- Builder.CreateStore(Result, ArgAddr);
-
- // Terminator
- Builder.CreateRetVoid();
-
- return Fn;
-}
-
-CallInst *InstrLowerer::getRMWOrCall(Value *Addr, Value *Val) {
- if (!RMWOrFunc)
- RMWOrFunc = createRMWOrFunc();
-
- return CallInst::Create(RMWOrFunc, {Addr, Val});
-}
-
Value *InstrLowerer::getBitmapAddress(InstrProfMCDCTVBitmapUpdate *I) {
auto *Bitmaps = getOrCreateRegionBitmaps(I);
if (!isRuntimeCounterRelocationEnabled())
@@ -1291,9 +1217,10 @@ void InstrLowerer::lowerCoverageData(GlobalVariable *CoverageNamesVar) {
void InstrLowerer::lowerMCDCTestVectorBitmapUpdate(
InstrProfMCDCTVBitmapUpdate *Update) {
+ auto &Ctx = M.getContext();
IRBuilder<> Builder(Update);
- auto *Int8Ty = Type::getInt8Ty(M.getContext());
- auto *Int32Ty = Type::getInt32Ty(M.getContext());
+ auto *Int8Ty = Type::getInt8Ty(Ctx);
+ auto *Int32Ty = Type::getInt32Ty(Ctx);
auto *MCDCCondBitmapAddr = Update->getMCDCCondBitmapAddr();
auto *BitmapAddr = getBitmapAddress(Update);
@@ -1321,7 +1248,36 @@ void InstrLowerer::lowerMCDCTestVectorBitmapUpdate(
// %7 = shl i8 1, %6
auto *ShiftedVal = Builder.CreateShl(Builder.getInt8(0x1), BitToSet);
- Builder.Insert(getRMWOrCall(BitmapByteAddr, ShiftedVal));
+ // Load profile bitmap byte.
+ // %mcdc.bits = load i8, ptr %4, align 1
+ auto *Bitmap = Builder.CreateLoad(Int8Ty, BitmapByteAddr, "mcdc.bits");
+
+ if (Options.Atomic || AtomicCounterUpdateAll) {
+ // If ((Bitmap & Val) != Val), then execute atomic (Bitmap |= Val).
+ // Note, just-loaded Bitmap might not be up-to-date. Use it just for
+ // early testing.
+ auto *Masked = Builder.CreateAnd(Bitmap, ShiftedVal);
+ auto *ShouldStore = Builder.CreateICmpNE(Masked, ShiftedVal);
+
+ // Assume updating will be rare.
+ auto *Unlikely = MDBuilder(Ctx).createUnlikelyBranchWeights();
+ Instruction *ThenBranch =
+ SplitBlockAndInsertIfThen(ShouldStore, Update, false, Unlikely);
+
+ // Execute if (unlikely(ShouldStore)).
+ Builder.SetInsertPoint(ThenBranch);
+ Builder.CreateAtomicRMW(AtomicRMWInst::Or, BitmapByteAddr, ShiftedVal,
+ MaybeAlign(), AtomicOrdering::Monotonic);
+ } else {
+ // Perform logical OR of profile bitmap byte and shifted bit offset.
+ // %8 = or i8 %mcdc.bits, %7
+ auto *Result = Builder.CreateOr(Bitmap, ShiftedVal);
+
+ // Store the updated profile bitmap byte.
+ // store i8 %8, ptr %3, align 1
+ Builder.CreateStore(Result, BitmapByteAddr);
+ }
+
Update->eraseFromParent();
}
diff --git a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
index fbdc7b57be4283..ee294a96b04bae 100644
--- a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
@@ -36,25 +36,21 @@ entry:
; CHECK-NEXT: %[[LAB8:[0-9]+]] = and i32 %[[TEMP]], 7
; CHECK-NEXT: %[[LAB9:[0-9]+]] = trunc i32 %[[LAB8]] to i8
; CHECK-NEXT: %[[LAB10:[0-9]+]] = shl i8 1, %[[LAB9]]
- ; CHECK-NEXT: call void @[[RMW_OR:.+]](ptr %[[LAB7]], i8 %[[LAB10]])
+ ; CHECK-NEXT: %[[BITS:.+]] = load i8, ptr %[[LAB7]], align 1
+ ; ATOMIC-NEXT: %[[MASKED:.+]] = and i8 %[[BITS]], %[[LAB10]]
+ ; ATOMIC-NEXT: %[[SHOULDWRITE:.+]] = icmp ne i8 %[[MASKED]], %[[LAB10]]
+ ; ATOMIC-NEXT: br i1 %[[SHOULDWRITE]], label %[[WRITE:.+]], label %[[SKIP:.+]], !prof ![[MDPROF:[0-9]+]]
+ ; ATOMIC: [[WRITE]]:
+ ; BASIC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
+ ; RELOC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
+ ; BASIC-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
+ ; RELOC-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
+ ; ATOMIC-NEXT: %{{.+}} = atomicrmw or ptr %[[LAB7]], i8 %[[LAB10]] monotonic, align 1
+ ; ATOMIC: [[SKIP]]:
ret void
+ ; CHECK-NEXT: ret void
}
-; CHECK: define private void @[[RMW_OR]](ptr %[[ARGPTR:.+]], i8 %[[ARGVAL:.+]])
-; CHECK: %[[BITS:.+]] = load i8, ptr %[[ARGPTR]], align 1
-; BASIC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[ARGVAL]]
-; RELOC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[ARGVAL]]
-; BASIC-NEXT: store i8 %[[LAB11]], ptr %[[ARGPTR]], align 1
-; RELOC-NEXT: store i8 %[[LAB11]], ptr %[[ARGPTR]], align 1
-; ATOMIC-NEXT: %[[MASKED:.+]] = and i8 %[[BITS]], %[[ARGVAL]]
-; ATOMIC-NEXT: %[[SHOULDWRITE:.+]] = icmp ne i8 %[[MASKED]], %[[ARGVAL]]
-; ATOMIC-NEXT: br i1 %[[SHOULDWRITE]], label %[[WRITE:.+]], label %[[SKIP:.+]], !prof ![[MDPROF:[0-9]+]]
-; ATOMIC: [[WRITE]]:
-; ATOMIC-NEXT: %{{.+}} = atomicrmw or ptr %[[ARGPTR]], i8 %[[ARGVAL]] monotonic, align 1
-; ATOMIC-NEXT: ret void
-; ATOMIC: [[SKIP]]:
-; CHECK-NEXT: ret void
-
; ATOMIC: ![[MDPROF]] = !{!"branch_weights", i32 1, i32 1048575}
declare void @llvm.instrprof.cover(ptr, i64, i32, i32)
``````````
</details>
https://github.com/llvm/llvm-project/pull/110792
More information about the llvm-commits
mailing list