-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[DebugInfo][RemoveDIs] Make getDbgValueRange inlineable #79331
Conversation
getDbgValueRange is the replacement of a common LLVM idiom of: 1) Am I currently looking at a DbgVariableIntrinsic instruction? 2) Let's do something special with it! We instead iterate over the range of DPValues attached to an instruction. Unfortunately in the common case of "there is no debug-info", this generates a spurious function call that's paid by non-debug builds. To get around this, make getDbgValueRange inlineable so that the "if (DbgMarker)" test can be inlined and guard the more expensive call. The false path should be optimisable-awayable to skipping the loop. However, due to header inclusion order we can't just make Instruction::getDbgValueRange inline because DPMarker hasn't been declared yet. So, define an inlinable function in the llvm:: namespace and pre-declare it -- the eventual code should be inlineable almost 100% of the time.
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Jeremy Morse (jmorse) Changes
We instead iterate over the range of DPValues attached to an instruction and do special things with those. Unfortunately in the common case of "there is no debug-info", this generates a spurious function call that's paid by non-debug builds. To get around this, make Full diff: https://github.com/llvm/llvm-project/pull/79331.diff 3 Files Affected:
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 737417fb9b9a542..0524a657bce4b60 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -453,6 +453,17 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DPValue &Value) {
return OS;
}
+/// Inline helper to return a range of DPValues attached to a marker. It needs
+/// to be inlined as it's frequently called, but also come after the declaration
+/// of DPMarker. Thus: it's pre-declared by users like Instruction, then an
+/// inlineable body defined here.
+inline iterator_range<simple_ilist<DPValue>::iterator>
+getDbgValueRange(DPMarker *DbgMarker) {
+ if (!DbgMarker)
+ return DPMarker::getEmptyDPValueRange();
+ return DbgMarker->getDbgValueRange();
+}
+
} // namespace llvm
#endif // LLVM_IR_DEBUGPROGRAMINSTRUCTION_H
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index fcd2ba838e7fd51..a785a43a0f11a6e 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -40,6 +40,8 @@ template <> struct ilist_alloc_traits<Instruction> {
static inline void deleteNode(Instruction *V);
};
+iterator_range<simple_ilist<DPValue>::iterator> getDbgValueRange(DPMarker *);
+
class Instruction : public User,
public ilist_node_with_parent<Instruction, BasicBlock,
ilist_iterator_bits<true>> {
@@ -76,7 +78,9 @@ class Instruction : public User,
bool InsertAtHead = false);
/// Return a range over the DPValues attached to this instruction.
- iterator_range<simple_ilist<DPValue>::iterator> getDbgValueRange() const;
+ iterator_range<simple_ilist<DPValue>::iterator> getDbgValueRange() const {
+ return llvm::getDbgValueRange(DbgMarker);
+ }
/// Return an iterator to the position of the "Next" DPValue after this
/// instruction, or std::nullopt. This is the position to pass to
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index d7bf1447921fec7..f9a38e48166c9ab 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -243,18 +243,6 @@ Instruction::cloneDebugInfoFrom(const Instruction *From,
return DbgMarker->cloneDebugInfoFrom(From->DbgMarker, FromHere, InsertAtHead);
}
-iterator_range<DPValue::self_iterator>
-Instruction::getDbgValueRange() const {
- BasicBlock *Parent = const_cast<BasicBlock *>(getParent());
- assert(Parent && "Instruction must be inserted to have DPValues");
- (void)Parent;
-
- if (!DbgMarker)
- return DPMarker::getEmptyDPValueRange();
-
- return DbgMarker->getDbgValueRange();
-}
-
std::optional<DPValue::self_iterator> Instruction::getDbgReinsertionPosition() {
// Is there a marker on the next instruction?
DPMarker *NextMarker = getParent()->getNextMarker(this);
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 😄
Thanks for the review! |
getDbgValueRange
is the replacement of a common LLVM idiom of:DbgVariableIntrinsic
instruction?We instead iterate over the range of DPValues attached to an instruction and do special things with those. Unfortunately in the common case of "there is no debug-info", this generates a spurious function call that's paid by non-debug builds.
To get around this, make
getDbgValueRange
inlineable so that the "if (DbgMarker)
" test can be inlined and guard the more expensive call. The false path should be optimisable-awayable to skipping the loop. However, due to header inclusion order we can't just makeInstruction::getDbgValueRange
inline becauseDPMarker
hasn't been declared yet. So, define an inlinable function in the llvm:: namespace and pre-declare it -- the eventual code should be inlineable almost 100% of the time.