Skip to content
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

[NFC][RemoveDIs] Add LocationType parameter to DPValue ctor #74091

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Dec 1, 2023

IMO we can tidy up the interfaces a bit once all intrinsics are supported, so we have a complete view.

@llvmbot
Copy link
Member

llvmbot commented Dec 1, 2023

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-ir

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

IMO we can tidy up the interfaces a bit once all intrinsics are supported, so we have a complete view.


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

2 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugProgramInstruction.h (+1-1)
  • (modified) llvm/lib/IR/DebugProgramInstruction.cpp (+2-3)
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index cd5d37a078015be..7f16151cdfc64ba 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -113,7 +113,7 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   /// Directly construct a new DPValue representing a dbg.value intrinsic
   /// assigning \p Location to the DV / Expr / DI variable.
   DPValue(Metadata *Location, DILocalVariable *DV, DIExpression *Expr,
-          const DILocation *DI);
+          const DILocation *DI, LocationType Type = LocationType::Value);
 
   /// Iterator for ValueAsMetadata that internally uses direct pointer iteration
   /// over either a ValueAsMetadata* or a ValueAsMetadata**, dereferencing to the
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 6a4ee9d6101076f..df45c6ea3a776fd 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -35,10 +35,9 @@ DPValue::DPValue(const DPValue &DPV)
       DbgLoc(DPV.getDebugLoc()), Type(DPV.getType()) {}
 
 DPValue::DPValue(Metadata *Location, DILocalVariable *DV, DIExpression *Expr,
-                 const DILocation *DI)
+                 const DILocation *DI, LocationType Type)
     : DebugValueUser(Location), Variable(DV), Expression(Expr), DbgLoc(DI),
-      Type(LocationType::Value) {
-}
+      Type(Type) {}
 
 void DPValue::deleteInstr() { delete this; }
 

Copy link
Contributor

@SLTozer SLTozer left a comment

Choose a reason for hiding this comment

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

SGTM, though although it might be a pain to implement, it might be better to have the LocationType argument be explicit - I've also got a patch to add static DPValue *createDP[Assign|Declare|Value]() functions though, so it might be better to leave the default so that we aren't rewriting all the new DPValue callsites twice.

@OCHyams
Copy link
Contributor Author

OCHyams commented Dec 8, 2023

I agree with both points; I think the result being we leave this patch as it is. Thanks!

@OCHyams OCHyams merged commit 4648acb into llvm:main Dec 11, 2023
@OCHyams OCHyams deleted the ddd/ctor branch December 11, 2023 12:24
OCHyams added a commit that referenced this pull request Dec 13, 2023
As part of the RemoveDIs project, transitioning to non-instruction debug
info, all debug intrinsic handling code needs to be duplicated to handle
DPValues.

--try-experimental-debuginfo-iterators enables the new debug mode in
tests if the CMake option has been enabled.

`getInsertPtAfterFramePtr` now returns an iterator so we don't lose
debug-info-communicating bits.

---

Depends on #73500, #74090, #74091.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants