Add new llvm.dbg.declare_value intrinsic. #168132
Open
+170
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-llvm-ir Author: Shubham Sandeep Rastogi (rastogishubham) ChangesFor swift async code, we need to use a debug intrinsic that behaves like an llvm.dbg.declare but can take any location type rather than just a pointer or integer. To solve this, a new debug instrinsic called llvm.dbg.declare_value has been created, which behaves exactly like an llvm.dbg.declare but can take non pointer and integer location types. More information here: https://discourse.llvm.org/t/rfc-introduce-new-llvm-dbg-coroframe-entry-intrinsic/88269 This is the first patch as part of a stack of patches Full diff: https://github.com/llvm/llvm-project/pull/168132.diff 17 Files Affected:
diff --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
index b0c5beae631ce..aa3d1d92548d7 100644
--- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -688,6 +688,8 @@ enum FunctionCodes {
FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE =
64, // [DILocation, DILocalVariable, DIExpression, Value]
FUNC_CODE_DEBUG_RECORD_LABEL = 65, // [DILocation, DILabel]
+ FUNC_CODE_DEBUG_RECORD_DECLARE_VALUE =
+ 66, // [DILocation, DILocalVariable, DIExpression, ValueAsMetadata]
};
enum UseListCodes {
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 4228ec9c3ef7a..49d9e686aeffa 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -1156,6 +1156,18 @@ namespace llvm {
DIExpression *Expr, const DILocation *DL,
InsertPosition InsertPt);
+ /// Insert a new llvm.dbg.declare_value intrinsic call.
+ /// \param Storage llvm::Value of the variable
+ /// \param VarInfo Variable's debug info descriptor.
+ /// \param Expr A complex location expression.
+ /// \param DL Debug info location.
+ /// \param InsertPt Location for the new intrinsic.
+ LLVM_ABI DbgInstPtr insertDeclareValue(llvm::Value *Storage,
+ DILocalVariable *VarInfo,
+ DIExpression *Expr,
+ const DILocation *DL,
+ InsertPosition InsertPt);
+
/// Insert a new llvm.dbg.label intrinsic call.
/// \param LabelInfo Label's debug info descriptor.
/// \param DL Debug info location.
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 862293c9666a7..ae738d8d6cd17 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -44,6 +44,8 @@ class Module;
LLVM_ABI TinyPtrVector<DbgVariableRecord *> findDVRDeclares(Value *V);
/// As above, for DVRValues.
LLVM_ABI TinyPtrVector<DbgVariableRecord *> findDVRValues(Value *V);
+/// As above, for DVRCoroFrameEntrys.
+LLVM_ABI TinyPtrVector<DbgVariableRecord *> findDVRDeclareValues(Value *V);
/// Finds the debug info records describing a value.
LLVM_ABI void
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 66f44fe34d3f6..f3d680e372d64 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -282,6 +282,7 @@ class DbgVariableRecord : public DbgRecord, protected DebugValueUser {
Declare,
Value,
Assign,
+ DeclareValue,
End, ///< Marks the end of the concrete types.
Any, ///< To indicate all LocationTypes in searches.
@@ -364,6 +365,13 @@ class DbgVariableRecord : public DbgRecord, protected DebugValueUser {
createDVRDeclare(Value *Address, DILocalVariable *DV, DIExpression *Expr,
const DILocation *DI, DbgVariableRecord &InsertBefore);
+ LLVM_ABI static DbgVariableRecord *
+ createDVRDeclareValue(Value *Address, DILocalVariable *DV, DIExpression *Expr,
+ const DILocation *DI);
+ LLVM_ABI static DbgVariableRecord *
+ createDVRDeclareValue(Value *Address, DILocalVariable *DV, DIExpression *Expr,
+ const DILocation *DI, DbgVariableRecord &InsertBefore);
+
/// Iterator for ValueAsMetadata that internally uses direct pointer iteration
/// over either a ValueAsMetadata* or a ValueAsMetadata**, dereferencing to the
/// ValueAsMetadata .
@@ -414,6 +422,7 @@ class DbgVariableRecord : public DbgRecord, protected DebugValueUser {
bool isDbgDeclare() const { return Type == LocationType::Declare; }
bool isDbgValue() const { return Type == LocationType::Value; }
+ bool isDbgDeclareValue() const { return Type == LocationType::DeclareValue; }
/// Get the locations corresponding to the variable referenced by the debug
/// info intrinsic. Depending on the intrinsic, this could be the
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 0622bfae2c845..589e2a65a6908 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -428,6 +428,7 @@ class DbgVariableIntrinsic : public DbgInfoIntrinsic {
case Intrinsic::dbg_declare:
case Intrinsic::dbg_value:
case Intrinsic::dbg_assign:
+ case Intrinsic::dbg_declare_value:
return true;
default:
return false;
@@ -464,6 +465,26 @@ class DbgDeclareInst : public DbgVariableIntrinsic {
/// @}
};
+/// This represents the llvm.dbg.declare_value instruction.
+class DbgDeclareValueInst : public DbgVariableIntrinsic {
+public:
+ Value *getAddress() const {
+ assert(getNumVariableLocationOps() == 1 &&
+ "dbg.declare_value must have exactly 1 location operand.");
+ return getVariableLocationOp(0);
+ }
+
+ /// \name Casting methods
+ /// @{
+ static bool classof(const IntrinsicInst *I) {
+ return I->getIntrinsicID() == Intrinsic::dbg_declare_value;
+ }
+ static bool classof(const Value *V) {
+ return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+ }
+ /// @}
+};
+
/// This represents the llvm.dbg.value instruction.
class DbgValueInst : public DbgVariableIntrinsic {
public:
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 07aa2faffa7c5..d95b053ca21bf 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1460,6 +1460,10 @@ let IntrProperties = [IntrNoMem, IntrSpeculatable] in {
llvm_metadata_ty]>;
def int_dbg_label : DefaultAttrsIntrinsic<[],
[llvm_metadata_ty]>;
+ def int_dbg_declare_value : DefaultAttrsIntrinsic<[],
+ [llvm_metadata_ty,
+ llvm_metadata_ty,
+ llvm_metadata_ty]>;
}
//===------------------ Exception Handling Intrinsics----------------------===//
diff --git a/llvm/lib/AsmParser/LLLexer.cpp b/llvm/lib/AsmParser/LLLexer.cpp
index 7a6c19ece92ac..940f7b53c8243 100644
--- a/llvm/lib/AsmParser/LLLexer.cpp
+++ b/llvm/lib/AsmParser/LLLexer.cpp
@@ -1005,6 +1005,7 @@ lltok::Kind LLLexer::LexIdentifier() {
DBGRECORDTYPEKEYWORD(declare);
DBGRECORDTYPEKEYWORD(assign);
DBGRECORDTYPEKEYWORD(label);
+ DBGRECORDTYPEKEYWORD(declare_value);
#undef DBGRECORDTYPEKEYWORD
if (Keyword.starts_with("DIFlag")) {
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 8e3ce4990f437..4e05610bc3e84 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -7152,7 +7152,8 @@ bool LLParser::parseDebugRecord(DbgRecord *&DR, PerFunctionState &PFS) {
.Case("declare", RecordKind::ValueKind)
.Case("value", RecordKind::ValueKind)
.Case("assign", RecordKind::ValueKind)
- .Case("label", RecordKind::LabelKind);
+ .Case("label", RecordKind::LabelKind)
+ .Case("declare_value", RecordKind::ValueKind);
// Parsing labels is trivial; parse here and early exit, otherwise go into the
// full DbgVariableRecord processing stage.
@@ -7177,7 +7178,8 @@ bool LLParser::parseDebugRecord(DbgRecord *&DR, PerFunctionState &PFS) {
LocType ValueType = StringSwitch<LocType>(Lex.getStrVal())
.Case("declare", LocType::Declare)
.Case("value", LocType::Value)
- .Case("assign", LocType::Assign);
+ .Case("assign", LocType::Assign)
+ .Case("declare_value", LocType::DeclareValue);
Lex.Lex();
if (parseToken(lltok::lparen, "Expected '(' here"))
diff --git a/llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp b/llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
index fe9e0ddca7091..911ec7501eb8b 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
@@ -272,6 +272,7 @@ GetCodeName(unsigned CodeID, unsigned BlockID,
STRINGIFY_CODE(FUNC_CODE, INST_CALLBR)
STRINGIFY_CODE(FUNC_CODE, BLOCKADDR_USERS)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_DECLARE)
+ STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_DECLARE_VALUE)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_VALUE)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_ASSIGN)
STRINGIFY_CODE(FUNC_CODE, DEBUG_RECORD_VALUE_SIMPLE)
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 8930d64de5e37..c2a0fb4f1ebfe 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6655,6 +6655,7 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
case bitc::FUNC_CODE_DEBUG_RECORD_VALUE_SIMPLE:
case bitc::FUNC_CODE_DEBUG_RECORD_VALUE:
case bitc::FUNC_CODE_DEBUG_RECORD_DECLARE:
+ case bitc::FUNC_CODE_DEBUG_RECORD_DECLARE_VALUE:
case bitc::FUNC_CODE_DEBUG_RECORD_ASSIGN: {
// DbgVariableRecords are placed after the Instructions that they are
// attached to.
@@ -6671,6 +6672,8 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
// ..., Value
// dbg_declare (FUNC_CODE_DEBUG_RECORD_DECLARE)
// ..., LocationMetadata
+ // dbg_declare_value (FUNC_CODE_DEBUG_RECORD_DECLARE_VALUE)
+ // ..., LocationMetadata
// dbg_assign (FUNC_CODE_DEBUG_RECORD_ASSIGN)
// ..., LocationMetadata, DIAssignID, DIExpression, LocationMetadata
unsigned Slot = 0;
@@ -6712,6 +6715,11 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
DVR = new DbgVariableRecord(RawLocation, Var, Expr, DIL,
DbgVariableRecord::LocationType::Declare);
break;
+ case bitc::FUNC_CODE_DEBUG_RECORD_DECLARE_VALUE:
+ DVR = new DbgVariableRecord(
+ RawLocation, Var, Expr, DIL,
+ DbgVariableRecord::LocationType::DeclareValue);
+ break;
case bitc::FUNC_CODE_DEBUG_RECORD_ASSIGN: {
DIAssignID *ID = cast<DIAssignID>(getFnMetadataByID(Record[Slot++]));
DIExpression *AddrExpr =
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 76494c792ac7b..1d0461478b90c 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -3844,6 +3844,9 @@ void ModuleBitcodeWriter::writeFunction(
} else if (DVR.isDbgDeclare()) {
Vals.push_back(VE.getMetadataID(DVR.getRawLocation()));
Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_RECORD_DECLARE, Vals);
+ } else if (DVR.isDbgDeclareValue()) {
+ Vals.push_back(VE.getMetadataID(DVR.getRawLocation()));
+ Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_RECORD_DECLARE_VALUE, Vals);
} else {
assert(DVR.isDbgAssign() && "Unexpected DbgRecord kind");
Vals.push_back(VE.getMetadataID(DVR.getRawLocation()));
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 4d4ffe93a8067..11b6c47219576 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -4837,6 +4837,9 @@ void AssemblyWriter::printDbgVariableRecord(const DbgVariableRecord &DVR) {
case DbgVariableRecord::LocationType::Declare:
Out << "declare";
break;
+ case DbgVariableRecord::LocationType::DeclareValue:
+ Out << "declare_value";
+ break;
case DbgVariableRecord::LocationType::Assign:
Out << "assign";
break;
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 58b7ddd0381e5..ed455cc5236de 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -1264,7 +1264,7 @@ static bool upgradeIntrinsicFunction1(Function *F, Function *&NewFn,
// Mark debug intrinsics for upgrade to new debug format.
if (CanUpgradeDebugIntrinsicsToRecords) {
if (Name == "addr" || Name == "value" || Name == "assign" ||
- Name == "declare" || Name == "label") {
+ Name == "declare" || Name == "label" || Name == "declare_value") {
// There's no function to replace these with.
NewFn = nullptr;
// But we do want these to get upgraded.
@@ -4632,6 +4632,11 @@ static void upgradeDbgIntrinsicToDbgRecord(StringRef Name, CallBase *CI) {
DbgVariableRecord::LocationType::Declare, unwrapMAVMetadataOp(CI, 0),
unwrapMAVOp(CI, 1), unwrapMAVOp(CI, 2), nullptr, nullptr, nullptr,
getDebugLocSafe(CI));
+ } else if (Name == "declare_value") {
+ DR = DbgVariableRecord::createUnresolvedDbgVariableRecord(
+ DbgVariableRecord::LocationType::DeclareValue,
+ unwrapMAVMetadataOp(CI, 0), unwrapMAVOp(CI, 1), unwrapMAVOp(CI, 2),
+ nullptr, nullptr, nullptr, getDebugLocSafe(CI));
} else if (Name == "addr") {
// Upgrade dbg.addr to dbg.value with DW_OP_deref.
MDNode *ExprNode = unwrapMAVOp(CI, 2);
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index ca11ecf2f473e..b01860dba74a9 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -1147,6 +1147,24 @@ DbgInstPtr DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo,
return DVR;
}
+DbgInstPtr DIBuilder::insertDeclareValue(Value *Storage,
+ DILocalVariable *VarInfo,
+ DIExpression *Expr,
+ const DILocation *DL,
+ InsertPosition InsertPt) {
+ assert(VarInfo &&
+ "empty or invalid DILocalVariable* passed to dbg.declare_value");
+ assert(DL && "Expected debug loc");
+ assert(DL->getScope()->getSubprogram() ==
+ VarInfo->getScope()->getSubprogram() &&
+ "Expected matching subprograms");
+
+ DbgVariableRecord *DVR =
+ DbgVariableRecord::createDVRDeclareValue(Storage, VarInfo, Expr, DL);
+ insertDbgVariableRecord(DVR, InsertPt);
+ return DVR;
+}
+
void DIBuilder::insertDbgVariableRecord(DbgVariableRecord *DVR,
InsertPosition InsertPt) {
assert(InsertPt.isValid());
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 0f9d064857dc4..22381208c4dd5 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -62,6 +62,23 @@ TinyPtrVector<DbgVariableRecord *> llvm::findDVRDeclares(Value *V) {
return Declares;
}
+TinyPtrVector<DbgVariableRecord *> llvm::findDVRDeclareValues(Value *V) {
+ // This function is hot. Check whether the value has any metadata to avoid a
+ // DenseMap lookup. This check is a bitfield datamember lookup.
+ if (!V->isUsedByMetadata())
+ return {};
+ auto *L = ValueAsMetadata::getIfExists(V);
+ if (!L)
+ return {};
+
+ TinyPtrVector<DbgVariableRecord *> Coros;
+ for (DbgVariableRecord *DVR : L->getAllDbgVariableRecordUsers())
+ if (DVR->getType() == DbgVariableRecord::LocationType::DeclareValue)
+ Coros.push_back(DVR);
+
+ return Coros;
+}
+
TinyPtrVector<DbgVariableRecord *> llvm::findDVRValues(Value *V) {
// This function is hot. Check whether the value has any metadata to avoid a
// DenseMap lookup. This check is a bitfield datamember lookup.
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 6b1fd3907dc41..7548902c5e154 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -43,6 +43,9 @@ DbgVariableRecord::DbgVariableRecord(const DbgVariableIntrinsic *DVI)
case Intrinsic::dbg_declare:
Type = LocationType::Declare;
break;
+ case Intrinsic::dbg_declare_value:
+ Type = LocationType::DeclareValue;
+ break;
case Intrinsic::dbg_assign: {
Type = LocationType::Assign;
const DbgAssignIntrinsic *Assign =
@@ -211,6 +214,22 @@ DbgVariableRecord::createDVRDeclare(Value *Address, DILocalVariable *DV,
return NewDVRDeclare;
}
+DbgVariableRecord *
+DbgVariableRecord::createDVRDeclareValue(Value *Address, DILocalVariable *DV,
+ DIExpression *Expr,
+ const DILocation *DI) {
+ return new DbgVariableRecord(ValueAsMetadata::get(Address), DV, Expr, DI,
+ LocationType::DeclareValue);
+}
+
+DbgVariableRecord *DbgVariableRecord::createDVRDeclareValue(
+ Value *Address, DILocalVariable *DV, DIExpression *Expr,
+ const DILocation *DI, DbgVariableRecord &InsertBefore) {
+ auto *NewDVRCoro = createDVRDeclareValue(Address, DV, Expr, DI);
+ NewDVRCoro->insertBefore(&InsertBefore);
+ return NewDVRCoro;
+}
+
DbgVariableRecord *DbgVariableRecord::createDVRAssign(
Value *Val, DILocalVariable *Variable, DIExpression *Expression,
DIAssignID *AssignID, Value *Address, DIExpression *AddressExpression,
@@ -418,6 +437,10 @@ DbgVariableRecord::createDebugIntrinsic(Module *M,
case DbgVariableRecord::LocationType::Declare:
IntrinsicFn = Intrinsic::getOrInsertDeclaration(M, Intrinsic::dbg_declare);
break;
+ case DbgVariableRecord::LocationType::DeclareValue:
+ IntrinsicFn =
+ Intrinsic::getOrInsertDeclaration(M, Intrinsic::dbg_declare_value);
+ break;
case DbgVariableRecord::LocationType::Value:
IntrinsicFn = Intrinsic::getOrInsertDeclaration(M, Intrinsic::dbg_value);
break;
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index fa18c3cd0f404..678b06a48e9d4 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -190,6 +190,9 @@ struct llvm::VerifierSupport {
case DbgVariableRecord::LocationType::Declare:
*OS << "declare";
break;
+ case DbgVariableRecord::LocationType::DeclareValue:
+ *OS << "declare_value";
+ break;
case DbgVariableRecord::LocationType::Assign:
*OS << "assign";
break;
@@ -7058,6 +7061,7 @@ void Verifier::visit(DbgVariableRecord &DVR) {
CheckDI(DVR.getType() == DbgVariableRecord::LocationType::Value ||
DVR.getType() == DbgVariableRecord::LocationType::Declare ||
+ DVR.getType() == DbgVariableRecord::LocationType::DeclareValue ||
DVR.getType() == DbgVariableRecord::LocationType::Assign,
"invalid #dbg record type", &DVR, DVR.getType(), BB, F);
|
rastogishubham
requested review from
OCHyams,
SLTozer,
adrian-prantl,
dwblaikie,
jmorse and
rnk
Collaborator
adrian-prantl
left a comment
adrian-prantl
left a comment
There was a problem hiding this comment.
I think this looks good — could you add a llvm-as | llvm-dis | llvm-as | llvm-dis roundtrip test to the IR Assembler test directory?
rastogishubham
force-pushed
the
DeclareValue
branch
from
3fdf562 to
0482bf6
Compare
Contributor
Author
Done! I am not sure what to check for, so I just checked that there was a #dbg_declare_value still present. Please let me know if I should change something |
🐧 Linux x64 Test Results
|
Collaborator
No, that's all that's needed to ensure serialization and deserialization doesn't lose any information. |
Member
|
Please also add this new record / intrinsic to the source-level debugging docs. |
jmorse
reviewed
Nov 18, 2025
Member
jmorse
left a comment
jmorse
left a comment
There was a problem hiding this comment.
I see this is adding some more intrinsic definitions -- could we avoid this, as open-source LLVM only supports debug records nowadays? If things like the Swift compiler still need intrinsics, this is awkward, especially as we keep on deleting bits of intrinsic support from LLVM.
If you're definitely keeping intrinsic support for the moment, it might want autoupgrade tests and an intrinsic round-trip test.
DbgVariableRecord has helpers like isAddressOfVariable and isValueOfVariable which are used in a few places in the codebase -- should declare_value return true for any of those? (This probably involves examining the callsites that deal with those).
Contributor
Author
@jmorse I only added the code for intrinsics for completeness, tbh, I looked at everywhere dbg_declare was defined and add code for declare_value, as far as I can see, swift only needs to call
Conceptually dbg_declare_value is supposed to be a dbg_declare but one that can hold a value rather than a ptr to a value, so Edit: I guess the intrinsic code is what is in |
Contributor
Author
|
@jryans thanks for pointing that out! |
rastogishubham
force-pushed
the
DeclareValue
branch
4 times, most recently
from
c0c23d5 to
a1b7e13
Compare
Contributor
felipepiovezan
left a comment
felipepiovezan
left a comment
There was a problem hiding this comment.
This LGTM (left some ideas for the docs).
To take a step back, and definitely outside of anything to be changed in this PR, why do we require the value to be in the entry BB? What if a variable has a single value in the entire function but its value is only available later?
llvm/docs/SourceLevelDebugging.rst
Outdated
| #dbg_declare_value([Value|MDNode], DILocalVariable, DIExpression, DILocation) | ||
| This record provides information about a local element (e.g., variable). The | ||
| first argument is is the new value, and in the function entry block. The |
Contributor
There was a problem hiding this comment.
the new value
It's weird to talk about the new value here, given that there is only ever one value. We also don't say what that value represents.
and in the function entry block
This reads a bit off, maybe we could add a "must be"?
So, to summarize all of the above, how about something like this:
The first argument, which must be a value in the function entry block, is used to compute the value of the variable throughout the entire function.
(note: I intentionally said "used to compute the value" instead of "is the value", as the actual value depends on the expression later)
Contributor
Author
There was a problem hiding this comment.
I broadly agree with the changes, also you are right about the value being in the entry BB, it doesn't have to be, there is nothing in the code that checks for that either. I have removed that from the docs as well
rastogishubham
force-pushed
the
DeclareValue
branch
2 times, most recently
from
c9475be to
a972e31
Compare
jmorse
reviewed
Nov 21, 2025
Member
jmorse
left a comment
jmorse
left a comment
There was a problem hiding this comment.
NB, force push not desired due to githubs propensity to throw away review comments (insert here, griping about phab).
Thanks for removing the intrinsic bits; this soft LGTMs, final say from Apple folks I guess.
| ret void, !dbg !21 | ||
| } | ||
|
|
||
| attributes #0 = { noinline nounwind optnone ssp uwtable(sync) "frame-pointer"="non-leaf-no-reserve" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="apple-m1" "target-features"="+aes,+altnzcv,+ccdp,+ccidx,+ccpp,+complxnum,+crc,+dit,+dotprod,+flagm,+fp-armv8,+fp16fml,+fptoint,+fullfp16,+jsconv,+lse,+neon,+pauth,+perfmon,+predres,+ras,+rcpc,+rdm,+sb,+sha2,+sha3,+specrestrict,+ssbs,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8a" } |
Member
There was a problem hiding this comment.
Typically we don't want to keep attributes around as they're a needless maintenance burden, unless they have an effect here.
Contributor
Author
There was a problem hiding this comment.
Yep, I removed them!
felipepiovezan
approved these changes
Nov 21, 2025
Collaborator
|
Thanks, LGTM! |
For swift async code, we need to use a debug intrinsic that behaves like an llvm.dbg.declare but can take any location type rather than just a pointer or integer. To solve this, a new debug instrinsic called llvm.dbg.declare_value has been created, which behaves exactly like an llvm.dbg.declare but can take non pointer and integer location types.
rastogishubham
force-pushed
the
DeclareValue
branch
from
a972e31 to
80114e1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For swift async code, we need to use a debug intrinsic that behaves like an llvm.dbg.declare but can take any location type rather than just a pointer or integer.
To solve this, a new debug instrinsic called llvm.dbg.declare_value has been created, which behaves exactly like an llvm.dbg.declare but can take non pointer and integer location types.
More information here: https://discourse.llvm.org/t/rfc-introduce-new-llvm-dbg-coroframe-entry-intrinsic/88269
This is the first patch as part of a stack of patches, with the one succeeding it being: #168134