From 85a7bba7d28365ff98dae74f20ebf9f53d42023a Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 16 Jun 2024 09:04:51 +0900 Subject: [PATCH] Cleanup MC/DC intrinsics for #82448 (#95496) 3rd arg of `tvbitmap.update` was made unused. Remove 3rd arg. Sweep `condbitmap.update`, since it is no longer used. --- clang/lib/CodeGen/CodeGenPGO.cpp | 3 +- clang/test/Profile/c-mcdc.c | 2 +- llvm/docs/LangRef.rst | 53 +------------------ llvm/include/llvm/IR/IntrinsicInst.h | 35 +----------- llvm/include/llvm/IR/Intrinsics.td | 7 +-- .../SelectionDAG/SelectionDAGBuilder.cpp | 2 - .../Instrumentation/InstrProfiling.cpp | 35 ------------ .../Instrumentation/InstrProfiling/mcdc.ll | 13 +---- llvm/unittests/IR/IntrinsicsTest.cpp | 3 -- 9 files changed, 9 insertions(+), 144 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 59139e342de886..2839697614595e 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -1260,9 +1260,8 @@ void CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder, // from a pointer to a dedicated temporary value on the stack that is itself // updated via emitMCDCCondBitmapReset() and emitMCDCCondBitmapUpdate(). The // index represents an executed test vector. - llvm::Value *Args[5] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy), + llvm::Value *Args[4] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy), Builder.getInt64(FunctionHash), - Builder.getInt32(0), // Unused Builder.getInt32(MCDCTestVectorBitmapOffset), MCDCCondBitmapAddr.emitRawPointer(CGF)}; Builder.CreateCall( diff --git a/clang/test/Profile/c-mcdc.c b/clang/test/Profile/c-mcdc.c index 251c18baa861dd..7c1d734028364f 100644 --- a/clang/test/Profile/c-mcdc.c +++ b/clang/test/Profile/c-mcdc.c @@ -82,7 +82,7 @@ int test(int a, int b, int c, int d, int e, int f) { // UPDATE FINAL BITMASK WITH RESULT. // NOPROFPASS-LABEL: lor.end: -// NOPROFPASS: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @__profn_test, i64 [[HASH]], i32 0, i32 0, ptr %mcdc.addr) +// NOPROFPASS: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @__profn_test, i64 [[HASH]], i32 0, ptr %mcdc.addr) // MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %mcdc.addr, align 4 // MCDC: %[[TEMP:[0-9]+]] = add i32 %[[TEMP0]], 0 // MCDC: %[[LAB1:[0-9]+]] = lshr i32 %[[TEMP]], 3 diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 6935ccdfc91961..63d188be2c1ce4 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -14441,52 +14441,6 @@ to generate the appropriate data structures and the code to instrument MC/DC test vectors in a format that can be written out by a compiler runtime and consumed via the ``llvm-profdata`` tool. -'``llvm.instrprof.mcdc.condbitmap.update``' Intrinsic -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Syntax: -""""""" - -:: - - declare void @llvm.instrprof.mcdc.condbitmap.update(ptr , i64 , - i32 , - ptr , - i1 ) - -Overview: -""""""""" - -The '``llvm.instrprof.mcdc.condbitmap.update``' intrinsic is used to track -MC/DC condition evaluation for each condition in a boolean expression. - -Arguments: -"""""""""" - -The first argument is a pointer to a global variable containing the -name of the entity being instrumented. This should generally be the -(mangled) function name for a set of counters. - -The second argument is a hash value that can be used by the consumer -of the profile data to detect changes to the instrumented source. - -The third argument is an ID of a condition to track. This value is used as a -bit index into the condition bitmap. - -The fourth argument is the address of the condition bitmap. - -The fifth argument is the boolean value representing the evaluation of the -condition (true or false) - -Semantics: -"""""""""" - -This intrinsic represents the update of a condition bitmap that is local to a -function and will cause the ``-instrprof`` pass to generate the code to -instrument the control flow around each condition in a boolean expression. The -ID of each condition corresponds to a bit index in the condition bitmap which -is set based on the evaluation of the condition. - '``llvm.instrprof.mcdc.tvbitmap.update``' Intrinsic ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -14496,7 +14450,6 @@ Syntax: :: declare void @llvm.instrprof.mcdc.tvbitmap.update(ptr , i64 , - i32 ) i32 , ptr ) @@ -14520,12 +14473,10 @@ name of the entity being instrumented. This should generally be the The second argument is a hash value that can be used by the consumer of the profile data to detect changes to the instrumented source. -The third argument is not used. - -The fourth argument is the bit index into the global test vector bitmap +The third argument is the bit index into the global test vector bitmap corresponding to the function. -The fifth argument is the address of the condition bitmap, which contains a +The fourth argument is the address of the condition bitmap, which contains a value representing an executed MC/DC test vector. It is loaded and used as the bit index of the test vector bitmap. diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h index 1ac4a5fffb43bb..3963a5c8ab8f99 100644 --- a/llvm/include/llvm/IR/IntrinsicInst.h +++ b/llvm/include/llvm/IR/IntrinsicInst.h @@ -1455,9 +1455,7 @@ class InstrProfInstBase : public IntrinsicInst { public: static bool classof(const Value *V) { if (const auto *Instr = dyn_cast(V)) - return isCounterBase(*Instr) || isMCDCBitmapBase(*Instr) || - Instr->getIntrinsicID() == - Intrinsic::instrprof_mcdc_condbitmap_update; + return isCounterBase(*Instr) || isMCDCBitmapBase(*Instr); return false; } // The name of the instrumented function. @@ -1618,43 +1616,14 @@ class InstrProfMCDCTVBitmapUpdate : public InstrProfMCDCBitmapInstBase { /// \return The index of the TestVector Bitmap upon which this intrinsic /// acts. ConstantInt *getBitmapIndex() const { - return cast(const_cast(getArgOperand(3))); + return cast(const_cast(getArgOperand(2))); } /// \return The address of the corresponding condition bitmap containing /// the index of the TestVector to update within the TestVector Bitmap. - Value *getMCDCCondBitmapAddr() const { - return cast(const_cast(getArgOperand(4))); - } -}; - -/// This represents the llvm.instrprof.mcdc.condbitmap.update intrinsic. -/// It does not pertain to global bitmap updates or parameters and so doesn't -/// inherit from InstrProfMCDCBitmapInstBase. -class InstrProfMCDCCondBitmapUpdate : public InstrProfInstBase { -public: - static bool classof(const IntrinsicInst *I) { - return I->getIntrinsicID() == Intrinsic::instrprof_mcdc_condbitmap_update; - } - static bool classof(const Value *V) { - return isa(V) && classof(cast(V)); - } - - /// \return The ID of the condition to update. - ConstantInt *getCondID() const { - return cast(const_cast(getArgOperand(2))); - } - - /// \return The address of the corresponding condition bitmap. Value *getMCDCCondBitmapAddr() const { return cast(const_cast(getArgOperand(3))); } - - /// \return The boolean value to set in the condition bitmap for the - /// corresponding condition ID. This represents how the condition evaluated. - Value *getCondBool() const { - return cast(const_cast(getArgOperand(4))); - } }; class PseudoProbeInst : public IntrinsicInst { diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td index 4c506a6ace23ea..ef500329d1fb9a 100644 --- a/llvm/include/llvm/IR/Intrinsics.td +++ b/llvm/include/llvm/IR/Intrinsics.td @@ -939,12 +939,7 @@ def int_instrprof_mcdc_parameters : Intrinsic<[], // A test vector bitmap update for instrumentation based MCDC profiling. def int_instrprof_mcdc_tvbitmap_update : Intrinsic<[], [llvm_ptr_ty, llvm_i64_ty, - llvm_i32_ty, llvm_i32_ty, llvm_ptr_ty]>; - -// A condition bitmap value update for instrumentation based MCDC profiling. -def int_instrprof_mcdc_condbitmap_update : Intrinsic<[], - [llvm_ptr_ty, llvm_i64_ty, - llvm_i32_ty, llvm_ptr_ty, llvm_i1_ty]>; + llvm_i32_ty, llvm_ptr_ty]>; def int_call_preallocated_setup : DefaultAttrsIntrinsic<[llvm_token_ty], [llvm_i32_ty]>; def int_call_preallocated_arg : DefaultAttrsIntrinsic<[llvm_ptr_ty], [llvm_token_ty, llvm_i32_ty]>; diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 98555b39db03c5..8838cce9810f8f 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -7576,8 +7576,6 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, llvm_unreachable("instrprof failed to lower mcdc parameters"); case Intrinsic::instrprof_mcdc_tvbitmap_update: llvm_unreachable("instrprof failed to lower an mcdc tvbitmap update"); - case Intrinsic::instrprof_mcdc_condbitmap_update: - llvm_unreachable("instrprof failed to lower an mcdc condbitmap update"); case Intrinsic::localescape: { MachineFunction &MF = DAG.getMachineFunction(); const TargetInstrInfo *TII = DAG.getSubtarget().getInstrInfo(); diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp index 0c79eaa812b5fc..9c34374f0ece19 100644 --- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp +++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -278,10 +278,6 @@ class InstrLowerer final { /// using the index represented by the a temp value into a bitmap. void lowerMCDCTestVectorBitmapUpdate(InstrProfMCDCTVBitmapUpdate *Ins); - /// Replace instrprof.mcdc.temp.update with a shift and or instruction using - /// the corresponding condition ID. - void lowerMCDCCondBitmapUpdate(InstrProfMCDCCondBitmapUpdate *Ins); - /// Compute the address of the counter value that this profiling instruction /// acts on. Value *getCounterAddress(InstrProfCntrInstBase *I); @@ -648,9 +644,6 @@ bool InstrLowerer::lowerIntrinsics(Function *F) { } else if (auto *IPBU = dyn_cast(&Instr)) { lowerMCDCTestVectorBitmapUpdate(IPBU); MadeChange = true; - } else if (auto *IPTU = dyn_cast(&Instr)) { - lowerMCDCCondBitmapUpdate(IPTU); - MadeChange = true; } } } @@ -1053,34 +1046,6 @@ void InstrLowerer::lowerMCDCTestVectorBitmapUpdate( Update->eraseFromParent(); } -void InstrLowerer::lowerMCDCCondBitmapUpdate( - InstrProfMCDCCondBitmapUpdate *Update) { - IRBuilder<> Builder(Update); - auto *Int32Ty = Type::getInt32Ty(M.getContext()); - auto *MCDCCondBitmapAddr = Update->getMCDCCondBitmapAddr(); - - // Load the MCDC temporary value from the stack. - // %mcdc.temp = load i32, ptr %mcdc.addr, align 4 - auto *Temp = Builder.CreateLoad(Int32Ty, MCDCCondBitmapAddr, "mcdc.temp"); - - // Zero-extend the evaluated condition boolean value (0 or 1) by 32bits. - // %1 = zext i1 %tobool to i32 - auto *CondV_32 = Builder.CreateZExt(Update->getCondBool(), Int32Ty); - - // Shift the boolean value left (by the condition's ID) to form a bitmap. - // %2 = shl i32 %1, getCondID()> - auto *ShiftedVal = Builder.CreateShl(CondV_32, Update->getCondID()); - - // Perform logical OR of the bitmap against the loaded MCDC temporary value. - // %3 = or i32 %mcdc.temp, %2 - auto *Result = Builder.CreateOr(Temp, ShiftedVal); - - // Store the updated temporary value back to the stack. - // store i32 %3, ptr %mcdc.addr, align 4 - Builder.CreateStore(Result, MCDCCondBitmapAddr); - Update->eraseFromParent(); -} - /// Get the name of a profiling variable for a particular function. static std::string getVarName(InstrProfInstBase *Inc, StringRef Prefix, bool &Renamed) { diff --git a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll index e9ae80891ea6e6..4980b45f90c50a 100644 --- a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll +++ b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll @@ -22,14 +22,7 @@ entry: %0 = load i32, ptr %A.addr, align 4 %tobool = icmp ne i32 %0, 0 - call void @llvm.instrprof.mcdc.condbitmap.update(ptr @__profn_test, i64 99278, i32 0, ptr %mcdc.addr, i1 %tobool) - ; CHECK: %[[TEMP:mcdc.*]] = load i32, ptr %mcdc.addr, align 4 - ; CHECK-NEXT: %[[LAB1:[0-9]+]] = zext i1 %tobool to i32 - ; CHECK-NEXT: %[[LAB2:[0-9]+]] = shl i32 %[[LAB1]], 0 - ; CHECK-NEXT: %[[LAB3:[0-9]+]] = or i32 %[[TEMP]], %[[LAB2]] - ; CHECK-NEXT: store i32 %[[LAB3]], ptr %mcdc.addr, align 4 - - call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @__profn_test, i64 99278, i32 1, i32 0, ptr %mcdc.addr) + call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @__profn_test, i64 99278, i32 0, ptr %mcdc.addr) ; CHECK: %[[TEMP0:mcdc.*]] = load i32, ptr %mcdc.addr, align 4 ; CHECK-NEXT: %[[TEMP:[0-9]+]] = add i32 %[[TEMP0]], 0 ; CHECK-NEXT: %[[LAB4:[0-9]+]] = lshr i32 %[[TEMP]], 3 @@ -47,6 +40,4 @@ declare void @llvm.instrprof.cover(ptr, i64, i32, i32) declare void @llvm.instrprof.mcdc.parameters(ptr, i64, i32) -declare void @llvm.instrprof.mcdc.condbitmap.update(ptr, i64, i32, ptr, i1) - -declare void @llvm.instrprof.mcdc.tvbitmap.update(ptr, i64, i32, i32, ptr) +declare void @llvm.instrprof.mcdc.tvbitmap.update(ptr, i64, i32, ptr) diff --git a/llvm/unittests/IR/IntrinsicsTest.cpp b/llvm/unittests/IR/IntrinsicsTest.cpp index dddd2f73d4446b..6f9e724c403266 100644 --- a/llvm/unittests/IR/IntrinsicsTest.cpp +++ b/llvm/unittests/IR/IntrinsicsTest.cpp @@ -77,7 +77,6 @@ TEST_F(IntrinsicsTest, InstrProfInheritance) { return isa(I) && is##PARENT(I); \ } __ISA(InstrProfCntrInstBase, InstrProfInstBase); - __ISA(InstrProfMCDCCondBitmapUpdate, InstrProfInstBase); __ISA(InstrProfCoverInst, InstrProfCntrInstBase); __ISA(InstrProfIncrementInst, InstrProfCntrInstBase); __ISA(InstrProfIncrementInstStep, InstrProfIncrementInst); @@ -96,8 +95,6 @@ TEST_F(IntrinsicsTest, InstrProfInheritance) { {Intrinsic::instrprof_increment, isInstrProfIncrementInst}, {Intrinsic::instrprof_increment_step, isInstrProfIncrementInstStep}, {Intrinsic::instrprof_callsite, isInstrProfCallsite}, - {Intrinsic::instrprof_mcdc_condbitmap_update, - isInstrProfMCDCCondBitmapUpdate}, {Intrinsic::instrprof_mcdc_parameters, isInstrProfMCDCBitmapParameters}, {Intrinsic::instrprof_mcdc_tvbitmap_update,