Skip to content

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Mar 1, 2024

No description provided.

@mtrofin mtrofin requested a review from ellishg March 1, 2024 01:08
@llvmbot llvmbot added the llvm:ir label Mar 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 1, 2024

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-ir

Author: Mircea Trofin (mtrofin)

Changes

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

1 Files Affected:

  • (modified) llvm/include/llvm/IR/IntrinsicInst.h (+16)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index fbaaef8ea44315..2bd4f94f53170e 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -1428,8 +1428,15 @@ class VACopyInst : public IntrinsicInst {
 };
 
 /// A base class for all instrprof intrinsics.
+class InstrProfInstBase;
+class InstrProfMCDCBitmapInstBase;
+class InstrProfMCDCCondBitmapUpdate;
 class InstrProfInstBase : public IntrinsicInst {
 public:
+  static bool classof(const Value *V) {
+    return isa<InstrProfInstBase>(V) || isa<InstrProfMCDCBitmapInstBase>(V) ||
+           isa<InstrProfMCDCCondBitmapUpdate>(V);
+  }
   // The name of the instrumented function.
   GlobalVariable *getName() const {
     return cast<GlobalVariable>(
@@ -1442,8 +1449,17 @@ class InstrProfInstBase : public IntrinsicInst {
 };
 
 /// A base class for all instrprof counter intrinsics.
+class InstProfCoverInst;
+class InstProfIncrementInst;
+class InstrProfTimestampInst;
+class InstrProfValueProfileInst;
 class InstrProfCntrInstBase : public InstrProfInstBase {
 public:
+  static bool classof(const Value *V){
+    return isa<InstProfCoverInst>(V) || isa<InstProfIncrementInst>(V) ||
+           isa<InstrProfTimestampInst>(V) || isa<InstrProfValueProfileInst>(V);
+  }
+
   // The number of counters for the instrumented function.
   ConstantInt *getNumCounters() const;
   // The index of the counter that this instruction acts on.

Copy link

github-actions bot commented Mar 1, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@mtrofin mtrofin changed the title [mfc] Fix RTTI for InstrProf intrinsics [nfc] Fix RTTI for InstrProf intrinsics Mar 1, 2024
@llvmbot llvmbot added PGO Profile Guided Optimizations llvm:transforms labels Mar 1, 2024
…#83364)

Simpler code, compared to tracking state of 2 variables and the ambiguity of "0" CountValue (is it 0 or is it invalid?)
@mtrofin
Copy link
Member Author

mtrofin commented Mar 5, 2024

@ellishg - is this good to go? thanks!

if (const auto *Instr = dyn_cast<IntrinsicInst>(V))
return isCounterBase(*Instr) || isMCDCBitmapBase(*Instr) ||
Instr->getIntrinsicID() ==
Intrinsic::instrprof_mcdc_condbitmap_update;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be in isMCDCBitmapBase() CC @evodius96

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, I found the comment for InstrProfMCDCCondBitmapUpdate

Copy link
Contributor

@ellishg ellishg left a comment

Choose a reason for hiding this comment

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

LGTM

@mtrofin mtrofin merged commit 3f7aa04 into llvm:main Mar 6, 2024
@mtrofin mtrofin deleted the rtti branch March 6, 2024 05:11
mtrofin added a commit to mtrofin/llvm-project that referenced this pull request Apr 20, 2024
Following up PR llvm#83511, added a test to cover the inheritance graph for these intrinsics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:ir llvm:transforms PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants