Page MenuHomePhabricator

debug-infoProject
ActivePublic

Recent Activity

Yesterday

aprantl updated the diff for D58534: [WIP] dsymutil support for DW_OP_convert.

Here's the the first part of the patch: support for inline locations. Support for location lists is still coming.

Fri, Feb 22, 5:51 PM · Restricted Project, debug-info
ABataev added a comment to D56587: Introduce DW_OP_LLVM_convert.

I don't think anyone mentioned it, that's why I was bringing it up - there's always an option to not render any location if it's not possible/worth the work. That's all I was asking - is it worth the complexity? (I wasn't sure anyone needed it - but sounds like Sony does, reckon it's worth the tradeoff in complexity in LLVM compared to the work required to support this in the Sony debugger?)

NVPTX also would need it, because they are stuck on DWARF v2.

Any ideas if NVPTX hit this case? my understanding was that NVPTX has a fairly restrictive set of code or actions that can be used.

Ping on this - still wondering if anyone needs the complicated code or if we could get away with the GNU extension + DWARFv5 standard

Fri, Feb 22, 4:40 PM · debug-info
dblaikie added a comment to D56587: Introduce DW_OP_LLVM_convert.

I don't think anyone mentioned it, that's why I was bringing it up - there's always an option to not render any location if it's not possible/worth the work. That's all I was asking - is it worth the complexity? (I wasn't sure anyone needed it - but sounds like Sony does, reckon it's worth the tradeoff in complexity in LLVM compared to the work required to support this in the Sony debugger?)

NVPTX also would need it, because they are stuck on DWARF v2.

Any ideas if NVPTX hit this case? my understanding was that NVPTX has a fairly restrictive set of code or actions that can be used.

Fri, Feb 22, 3:53 PM · debug-info
lebedev.ri added inline comments to D58035: [clang/DIVar] Emit flag for params that have unchanged values.
Fri, Feb 22, 6:58 AM · debug-info
riccibruno added inline comments to D58035: [clang/DIVar] Emit flag for params that have unchanged values.
Fri, Feb 22, 6:51 AM · debug-info
djtodoro updated the diff for D58035: [clang/DIVar] Emit flag for params that have unchanged values.
  • Add a field in ParmVarDecl instead of VarDecl
  • Use a bit in ParmVarDeclBitfields to indicate parameter modification
  • Add support for the bit in ASTReaderDecl.cpp / ASTWriterDecl.cpp
  • Add test case for templates
Fri, Feb 22, 6:31 AM · debug-info
djtodoro added a comment to D58035: [clang/DIVar] Emit flag for params that have unchanged values.

@riccibruno Thanks for your comments!

Fri, Feb 22, 6:27 AM · debug-info
djtodoro added a comment to D58035: [clang/DIVar] Emit flag for params that have unchanged values.

I was under the impression that space inside VarDecl was quite constrained. Pardon the likely naive question, but: is there any way to make the representation more compact (maybe sneak a bit into ParmVarDeclBitfields)?

@vsk Thanks for the comment! Sure, it is better idea. Initially, we thought this could be used even for local variables, but since we are using it only for parameters it makes more sense.

Fri, Feb 22, 6:22 AM · debug-info
markus added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Fri, Feb 22, 5:08 AM · debug-info
markus updated the diff for D56587: Introduce DW_OP_LLVM_convert.
  • Added test with two CUs.
  • Updated some comments.
  • Removed the -generate-typed-dwarf5-expr option, will only produce DW_OP_convert for Dwarf v5, for all prior versions the mask & shift expression is used. I suggest that we forget about DW_OP_GNU_convert and debugger tunings for this review. If that is really desired it can be added later in a separate patch.
Fri, Feb 22, 4:52 AM · debug-info
riccibruno added a comment to D58035: [clang/DIVar] Emit flag for params that have unchanged values.

I am wondering about the semantics of this bit. I don't think that you can know for sure within clang whether a variable has been modified. The best you can do is know for sure that some variable has been modified, but I don't think you can prove that it has not been modified.

Fri, Feb 22, 4:04 AM · debug-info
djtodoro added inline comments to D58044: [DwarfDebug] Dump call site debug info into DWARF.
Fri, Feb 22, 1:07 AM · debug-info

Thu, Feb 21

aprantl added a parent revision for D56587: Introduce DW_OP_LLVM_convert: D58534: [WIP] dsymutil support for DW_OP_convert.
Thu, Feb 21, 5:13 PM · debug-info
aprantl added a child revision for D58534: [WIP] dsymutil support for DW_OP_convert: D56587: Introduce DW_OP_LLVM_convert.
Thu, Feb 21, 5:13 PM · Restricted Project, debug-info
aprantl created D58534: [WIP] dsymutil support for DW_OP_convert.
Thu, Feb 21, 5:12 PM · Restricted Project, debug-info
aprantl added a comment to D56587: Introduce DW_OP_LLVM_convert.

Could you please add a third testcase with two DICompileUnits, both of which have a DIExpression with a DW_OP_LLVM_convert that will use the same type and then verify that we emit the type in both CUs? This will help guard against later modifications breaking this scenario.

Thu, Feb 21, 3:07 PM · debug-info
riccibruno added a comment to D58035: [clang/DIVar] Emit flag for params that have unchanged values.

Oh and I think that you will also have to update the serialization code/de-serialization code in ASTReaderDecl.cpp / ASTWriterDecl.cpp. You might also have to update TreeTransform but I am less familiar with this.

Thu, Feb 21, 1:40 PM · debug-info
riccibruno added a comment to D58035: [clang/DIVar] Emit flag for params that have unchanged values.

If this bit is relevant to function parameters, why is getIsArgumentModified in VarDecl and not in ParamVarDecl ? What you can do is move the relevant methods to ParamVarDecl, and stash the bit in ParmVarDeclBitfields.

Thu, Feb 21, 1:12 PM · debug-info
riccibruno edited reviewers for D58035: [clang/DIVar] Emit flag for params that have unchanged values, added: riccibruno; removed: bricci.
Thu, Feb 21, 1:04 PM · debug-info
probinson added inline comments to D58044: [DwarfDebug] Dump call site debug info into DWARF.
Thu, Feb 21, 11:55 AM · debug-info
vsk added inline comments to D58044: [DwarfDebug] Dump call site debug info into DWARF.
Thu, Feb 21, 11:46 AM · debug-info
probinson added inline comments to D58044: [DwarfDebug] Dump call site debug info into DWARF.
Thu, Feb 21, 11:38 AM · debug-info
probinson added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Thu, Feb 21, 11:31 AM · debug-info
vsk added inline comments to D58044: [DwarfDebug] Dump call site debug info into DWARF.
Thu, Feb 21, 11:26 AM · debug-info
vsk added reviewers for D58035: [clang/DIVar] Emit flag for params that have unchanged values: bricci, rsmith.

+ rsmith, + bricci for review.

Thu, Feb 21, 11:02 AM · debug-info
probinson added inline comments to D58035: [clang/DIVar] Emit flag for params that have unchanged values.
Thu, Feb 21, 10:36 AM · debug-info
aprantl added a comment to D56587: Introduce DW_OP_LLVM_convert.

There's a nonzero chance that this patch will break llvm/tools/dsymutil. I'll see if I can come up with a testcase for that (hopefully later today).

Thu, Feb 21, 8:39 AM · debug-info
markus updated the diff for D56587: Introduce DW_OP_LLVM_convert.
  • updated to top of trunk to have the llvm-dwarfdump updates committed earlier today
  • updated tests and made additional improvements to llvm-dwarfdump
  • fixed segfault bug where we crashed on release builds but not debug builds (the allocation of DIEBaseTypeRef)
Thu, Feb 21, 5:50 AM · debug-info

Wed, Feb 20

markus added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Wed, Feb 20, 11:21 AM · debug-info
aprantl added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Wed, Feb 20, 8:16 AM · debug-info
markus updated the diff for D56587: Introduce DW_OP_LLVM_convert.

+ Addressed review remark (use range-based loop instead of iterators).
+ Updated tests (probably forgot during last rebase. Verified the values used now with a clean master)

Wed, Feb 20, 7:05 AM · debug-info
markus added a comment to D56587: Introduce DW_OP_LLVM_convert.

I have created a review for the requested 'llvm-dwarfdump' updates (the printing part) in https://reviews.llvm.org/D58442

Wed, Feb 20, 6:10 AM · debug-info
markus added a comment to D56587: Introduce DW_OP_LLVM_convert.

I'm a touch confused by the whole discussion here - so I'll write some things & perhaps folks can correct my understanding where necessary.

I think that we all are. Maybe it is because we tried some many different approaches in the same review that it is a bit convoluted now.

The issue is that a location expression (either in a direct location, or in a loclist) needs to reference a DIE.

Correct.

Sounds like that DIE reference is necessarily CU-local (because we're talking about precomputing the offset - and we could only do that if it's CU-local).

Also correct. The spec says that the reference is an offset into the current CU.

We already emit other CU-local DIE references in attributes (eg: DW_AT_specification, etc) references with hardcoded 4 bytes in size- so why would it be problematic to emit this one in the same way (with a padded ULEB128 that we know will give us 4 bytes of offset to work with)?

Agree. It should be no more problematic than what is done in other places already. The 4 byte ULEB128 gives us 28 bits to encode the offset in but as reasoned in other comments there is no way that limit can be reached.

& yeah, maybe if someone wants to save us some size (at the cost of whatever computational complexity this invokes in the assembler) it could be replaced with label differences (doing label differences for all DIE references would be nifty for manual editing/mucking about with DWARF, but not a big deal).

Agree.

Wed, Feb 20, 6:07 AM · debug-info

Tue, Feb 19

dblaikie added a comment to D56587: Introduce DW_OP_LLVM_convert.

I'm a touch confused by the whole discussion here - so I'll write some things & perhaps folks can correct my understanding where necessary.

Tue, Feb 19, 11:18 AM · debug-info
aprantl added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Tue, Feb 19, 10:34 AM · debug-info
markus added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Tue, Feb 19, 10:25 AM · debug-info
aprantl added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Tue, Feb 19, 8:52 AM · debug-info
aprantl added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Tue, Feb 19, 8:45 AM · debug-info
aprantl added a reviewer for D56587: Introduce DW_OP_LLVM_convert: dblaikie.
Tue, Feb 19, 8:45 AM · debug-info
aprantl added a comment to D58034: [IR/DIVar] Add flag for params that have unchanged values.

Looking good, thanks!

Tue, Feb 19, 8:39 AM · debug-info
markus added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Tue, Feb 19, 2:30 AM · debug-info
markus added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Tue, Feb 19, 1:38 AM · debug-info
djtodoro added inline comments to D58042: [LiveDebugValues] Emit parameter's entry value.
Tue, Feb 19, 12:15 AM · debug-info
djtodoro updated the diff for D58034: [IR/DIVar] Add flag for params that have unchanged values.
  • Adjust documentation
Tue, Feb 19, 12:05 AM · debug-info

Mon, Feb 18

aprantl added inline comments to D56587: Introduce DW_OP_LLVM_convert.
Mon, Feb 18, 10:13 AM · debug-info
aprantl accepted D58224: [DebugInfo] Adjust fragment offset for big endian targets when splitting alloca in SROA.

LGTM with update to testcase.

Mon, Feb 18, 9:47 AM · debug-info, Restricted Project
aprantl added inline comments to D58034: [IR/DIVar] Add flag for params that have unchanged values.
Mon, Feb 18, 9:22 AM · debug-info
alexey.lapshin updated the diff for D58194: [DebugInfo] add SectionedAddress to DebugInfo interfaces..

deleted LLD test as it was requested. Modified llvm-objdump test to not to use clang.

Mon, Feb 18, 6:17 AM · debug-info, Restricted Project
dstenb added inline comments to D58042: [LiveDebugValues] Emit parameter's entry value.
Mon, Feb 18, 5:08 AM · debug-info
markus updated the diff for D56587: Introduce DW_OP_LLVM_convert.
  • Did some cleanup
  • Added tests
  • Emit DW_OP_convert for Dwarf5
  • Emit legacy shift and mask expression for Dwarf4 and lower
  • Added llc option -generate-typed-dwarf5-expr to force emission regardless of targeted Dwarf version
Mon, Feb 18, 2:35 AM · debug-info