aprantl added inline comments.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1288
DwarfExpr.addFragmentOffset(DIExpr);
- if (Location.isIndirect())
+ if (Location.isIndirect() && !DIExpr->isEntryValue())
DwarfExpr.setMemoryLocationKind();
----------------
Can you add a comment? it's not necessarily obvious why an indirect entry value
is not a memory location. Is it because an entry value is its own kind?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:790
+ // FIXME: This produces unusable descriptions when the register contains
+ // a pointer to a temporary copy of a struct passed by value.
DIExpression *EntryExpr = DIExpression::get(
----------------
What does "unusable" mean? Incorrect? Invalid?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2404
MachineLocation Location = Value.getLoc();
- if (Location.isIndirect())
+ if (Location.isIndirect() && !DIExpr->isEntryValue())
DwarfExpr.setMemoryLocationKind();
----------------
same here
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2412
}
const TargetRegisterInfo &TRI = *AP.MF->getSubtarget().getRegisterInfo();
----------------
factoring out this snippet into a template and reusing it in
DwarfCompileUnit.cpp is probably not going to make this more readable, is it?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:146
/// The flags of location description being produced.
+ enum {
----------------
This comment is weird :-)
`Additional flags that may be combined with any location description kind.`?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:149
+ EntryValue = 1 << 0,
+ IndirectEntryValue = 1 << 1,
+ CallSiteParamValue = 1 << 2
----------------
Would it make more sense call this `Indirect` (w/o EntryValue) and treat it as
a separate bit, so you can still test for EntryValue independently?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:308
/// Lock this down to become an entry value location.
- void setEntryValueFlag() { LocationFlags |= EntryValue; }
+ void setEntryValueFlagFromLoc(MachineLocation Loc) {
+ LocationFlags |= Loc.isIndirect() ? IndirectEntryValue : EntryValue;
----------------
Is the dependency on MachineLocation really necessary, or could this just take
a bool / enum IsIndirect?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:309
+ void setEntryValueFlagFromLoc(MachineLocation Loc) {
+ LocationFlags |= Loc.isIndirect() ? IndirectEntryValue : EntryValue;
+ }
----------------
see comment in header...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80345/new/
https://reviews.llvm.org/D80345
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
