From 848deeaf16a1d8a54bbeb6256f2a4a85122703c9 Mon Sep 17 00:00:00 2001 From: Mircea Trofin Date: Wed, 24 Apr 2024 07:51:55 -0700 Subject: [PATCH 1/5] [llvm][ctx_profile] Add the `llvm.instrprof.callsite` intrinsic (Tracking Issue: #89287, RFC referenced there) --- llvm/include/llvm/IR/IntrinsicInst.h | 16 ++++++++++++++++ llvm/include/llvm/IR/Intrinsics.td | 5 +++++ llvm/unittests/IR/IntrinsicsTest.cpp | 2 ++ 3 files changed, 23 insertions(+) diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h index 4f22720f1c558..8a137ffe28b9f 100644 --- a/llvm/include/llvm/IR/IntrinsicInst.h +++ b/llvm/include/llvm/IR/IntrinsicInst.h @@ -1435,6 +1435,7 @@ class InstrProfInstBase : public IntrinsicInst { case Intrinsic::instrprof_cover: case Intrinsic::instrprof_increment: case Intrinsic::instrprof_increment_step: + case Intrinsic::instrprof_callsite: case Intrinsic::instrprof_timestamp: case Intrinsic::instrprof_value_profile: return true; @@ -1519,6 +1520,21 @@ class InstrProfIncrementInstStep : public InstrProfIncrementInst { } }; +/// This represents the llvm.instrprof.increment intrinsic. +/// It is structurally like the increment or step counters, hence the +/// inheritance relationship, albeit somewhat tenuous (it's not 'counting' per +/// se) +class InstrProfCallsite : public InstrProfCntrInstBase { +public: + static bool classof(const IntrinsicInst *I) { + return I->getIntrinsicID() == Intrinsic::instrprof_callsite; + } + static bool classof(const Value *V) { + return isa(V) && classof(cast(V)); + } + Value *getCallee() const; +}; + /// This represents the llvm.instrprof.timestamp intrinsic. class InstrProfTimestampInst : public InstrProfCntrInstBase { public: diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td index 1d20f7e1b1985..a14e9dedef8c9 100644 --- a/llvm/include/llvm/IR/Intrinsics.td +++ b/llvm/include/llvm/IR/Intrinsics.td @@ -914,6 +914,11 @@ def int_instrprof_increment_step : Intrinsic<[], [llvm_ptr_ty, llvm_i64_ty, llvm_i32_ty, llvm_i32_ty, llvm_i64_ty]>; +// Callsite instrumentation for contextual profiling +def int_instrprof_callsite : Intrinsic<[], + [llvm_ptr_ty, llvm_i64_ty, + llvm_i32_ty, llvm_i32_ty, llvm_ptr_ty]>; + // A timestamp for instrumentation based profiling. def int_instrprof_timestamp : Intrinsic<[], [llvm_ptr_ty, llvm_i64_ty, llvm_i32_ty, llvm_i32_ty]>; diff --git a/llvm/unittests/IR/IntrinsicsTest.cpp b/llvm/unittests/IR/IntrinsicsTest.cpp index 3fa4b2cf73b6b..dddd2f73d4446 100644 --- a/llvm/unittests/IR/IntrinsicsTest.cpp +++ b/llvm/unittests/IR/IntrinsicsTest.cpp @@ -81,6 +81,7 @@ TEST_F(IntrinsicsTest, InstrProfInheritance) { __ISA(InstrProfCoverInst, InstrProfCntrInstBase); __ISA(InstrProfIncrementInst, InstrProfCntrInstBase); __ISA(InstrProfIncrementInstStep, InstrProfIncrementInst); + __ISA(InstrProfCallsite, InstrProfCntrInstBase); __ISA(InstrProfTimestampInst, InstrProfCntrInstBase); __ISA(InstrProfValueProfileInst, InstrProfCntrInstBase); __ISA(InstrProfMCDCBitmapInstBase, InstrProfInstBase); @@ -94,6 +95,7 @@ TEST_F(IntrinsicsTest, InstrProfInheritance) { {Intrinsic::instrprof_cover, isInstrProfCoverInst}, {Intrinsic::instrprof_increment, isInstrProfIncrementInst}, {Intrinsic::instrprof_increment_step, isInstrProfIncrementInstStep}, + {Intrinsic::instrprof_callsite, isInstrProfCallsite}, {Intrinsic::instrprof_mcdc_condbitmap_update, isInstrProfMCDCCondBitmapUpdate}, {Intrinsic::instrprof_mcdc_parameters, From 622bba5b4fad8f7004b5e5dabd70520a0b956852 Mon Sep 17 00:00:00 2001 From: Mircea Trofin Date: Wed, 24 Apr 2024 10:54:16 -0700 Subject: [PATCH 2/5] fix comment, added LangRef. --- llvm/docs/LangRef.rst | 31 ++++++++++++++++++++++++++++ llvm/include/llvm/IR/IntrinsicInst.h | 2 +- llvm/lib/IR/IntrinsicInst.cpp | 4 ++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 0e87a8e2ace0e..d90b5d0a6e743 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -14139,6 +14139,37 @@ Semantics: """""""""" See description of '``llvm.instrprof.increment``' intrinsic. +'``llvm.instrprof.callsite``' Intrinsic +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Syntax: +""""""" + +:: + + declare void @llvm.instrprof.callsite(ptr , i64 , + i32 , + i32 , ptr ) + +Overview: +""""""""" + +The '``llvm.instrprof.callsite``' intrinsic should be emitted before a callsite +that's not to a "fake" callee (like another intrinsic or asm). + +Arguments: +"""""""""" +The first 3 arguments are similar to ``llvm.instrprof.increment``. The indexing +is specific to callsites, meaning callsites are indexed from 0, independent from +the indexes used by the other intrinsics (such as +``llvm.instrprof.increment[.step]``). + +The last argument is the called value of the callsite this intrinsic precedes. + +Semantics: +"""""""""" +This is lowered by contextual profiling. + '``llvm.instrprof.timestamp``' Intrinsic ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h index 8a137ffe28b9f..081e72cc82b2c 100644 --- a/llvm/include/llvm/IR/IntrinsicInst.h +++ b/llvm/include/llvm/IR/IntrinsicInst.h @@ -1520,7 +1520,7 @@ class InstrProfIncrementInstStep : public InstrProfIncrementInst { } }; -/// This represents the llvm.instrprof.increment intrinsic. +/// This represents the llvm.instrprof.callsite intrinsic. /// It is structurally like the increment or step counters, hence the /// inheritance relationship, albeit somewhat tenuous (it's not 'counting' per /// se) diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp index 89403e1d7fcb4..9bf8dae89458f 100644 --- a/llvm/lib/IR/IntrinsicInst.cpp +++ b/llvm/lib/IR/IntrinsicInst.cpp @@ -291,6 +291,10 @@ Value *InstrProfIncrementInst::getStep() const { return ConstantInt::get(Type::getInt64Ty(Context), 1); } +Value *InstrProfCallsite::getCallee() const { + return getArgOperand(4); +} + std::optional ConstrainedFPIntrinsic::getRoundingMode() const { unsigned NumOperands = arg_size(); Metadata *MD = nullptr; From fca27feb52becfb9b1ebaf9c18f428d54c6e0290 Mon Sep 17 00:00:00 2001 From: Mircea Trofin Date: Wed, 24 Apr 2024 14:29:52 -0700 Subject: [PATCH 3/5] format --- llvm/lib/IR/IntrinsicInst.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp index 9bf8dae89458f..9ee6dae30cac2 100644 --- a/llvm/lib/IR/IntrinsicInst.cpp +++ b/llvm/lib/IR/IntrinsicInst.cpp @@ -291,9 +291,7 @@ Value *InstrProfIncrementInst::getStep() const { return ConstantInt::get(Type::getInt64Ty(Context), 1); } -Value *InstrProfCallsite::getCallee() const { - return getArgOperand(4); -} +Value *InstrProfCallsite::getCallee() const { return getArgOperand(4); } std::optional ConstrainedFPIntrinsic::getRoundingMode() const { unsigned NumOperands = arg_size(); From b7a042d276cb8e7dd61fc2079629e9884e4da09a Mon Sep 17 00:00:00 2001 From: Mircea Trofin Date: Thu, 25 Apr 2024 09:28:13 -0700 Subject: [PATCH 4/5] addressed some comments --- llvm/docs/LangRef.rst | 2 +- llvm/lib/IR/IntrinsicInst.cpp | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index d90b5d0a6e743..f90503a03a5a8 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -14159,7 +14159,7 @@ that's not to a "fake" callee (like another intrinsic or asm). Arguments: """""""""" -The first 3 arguments are similar to ``llvm.instrprof.increment``. The indexing +The first 4 arguments are similar to ``llvm.instrprof.increment``. The indexing is specific to callsites, meaning callsites are indexed from 0, independent from the indexes used by the other intrinsics (such as ``llvm.instrprof.increment[.step]``). diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp index 9ee6dae30cac2..8faeb4e9951f7 100644 --- a/llvm/lib/IR/IntrinsicInst.cpp +++ b/llvm/lib/IR/IntrinsicInst.cpp @@ -291,7 +291,11 @@ Value *InstrProfIncrementInst::getStep() const { return ConstantInt::get(Type::getInt64Ty(Context), 1); } -Value *InstrProfCallsite::getCallee() const { return getArgOperand(4); } +Value *InstrProfCallsite::getCallee() const { + if (isa(this)) + return getArgOperand(4); + return nullptr; +} std::optional ConstrainedFPIntrinsic::getRoundingMode() const { unsigned NumOperands = arg_size(); From 2a7dd97a32398cfd9078ec2f012ce3b54a9bf7ea Mon Sep 17 00:00:00 2001 From: Mircea Trofin Date: Thu, 25 Apr 2024 10:00:50 -0700 Subject: [PATCH 5/5] `fixme` in docs --- llvm/docs/LangRef.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index f90503a03a5a8..f169ab941c457 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -14154,8 +14154,10 @@ Syntax: Overview: """"""""" +.. FIXME: detail when it's emitted once the support is added + The '``llvm.instrprof.callsite``' intrinsic should be emitted before a callsite -that's not to a "fake" callee (like another intrinsic or asm). +that's not to a "fake" callee (like another intrinsic or asm). Arguments: """""""""" @@ -14168,6 +14170,8 @@ The last argument is the called value of the callsite this intrinsic precedes. Semantics: """""""""" +.. FIXME: detail how when the lowering pass is added. + This is lowered by contextual profiling. '``llvm.instrprof.timestamp``' Intrinsic