- User Since
- Nov 19 2018, 1:12 AM (22 w, 15 h)
Thu, Apr 18
! In D59687#1468571, @probinson wrote:
It sounds like you're trying to do this with aggregate types, and that won't work. Only base types (for DWARF 5) or address-like types (for DWARF <= 4) should end up on the stack.
Tue, Apr 16
The Chromium compilation segfault comes down to two issues:
Sat, Apr 13
Has been reverted in
commit 0366e3e18142466e4dd99d3109d8facd093cedc8 (llvm.org/master, master) Author: Hans Wennborg <firstname.lastname@example.org> Date: Fri Apr 12 12:54:52 2019 +0000
Fri, Apr 12
Thu, Apr 11
Moved isImplicit() to .cpp file.
Tue, Apr 9
Rebased and updated isImplicit() to only check last elements.
Fri, Apr 5
Ping @aprantl, just a friendly reminder.
Thu, Mar 28
Rebased and fixed style nit.
Moved isImplicit to DIExpression class and updated test to do sext instead of zext.
Tue, Mar 26
Again (and sorry for possibly over emphasizing this) the purpose of this patch is that a DW_OP_deref needs to be inserted before a memory location description can be prepended to an already existing implicit location description as otherwise the latter expression would act on the former address and not the actual value in memory.
Mon, Mar 25
I am a bit worried about the other DW_OP_deref used in the compiler and how they behave wrt big-endian targets when loading something that is smaller than the size of an address on the target machine. I do not have any concrete example of where it goes wrong but I have not actively looked for one either.
Mar 22 2019
Mar 20 2019
Mar 19 2019
I guess what we want to achieve here (and please correct me if I am wrong) is
Mar 12 2019
Any improvement is an improvement so I am happy with that but it is still mentioned that this solution is a hack and I guess the
Mar 1 2019
We're almost there!
Feb 28 2019
- Removed explicit check for DW_OP_convert and replaced with a generic handling of operands that should work with all types (regardless if it is first, second or both that is BaseTypeRef).
Feb 27 2019
- Rewrote the expression parsing state machine in DwarfDebug::emitDebugLocEntry to use DWARFExpression as this avoids duplication of code describing ops and their arguments (got inspired by the dsymutil review).
Feb 26 2019
Feb 25 2019
Feb 22 2019
- 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.
Feb 21 2019
- 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)
Feb 20 2019
Added support for rudimentary verify of expressions (basically to check that a BaseTypeRef operand actually points to a DW_TAG_base_type DIE).
+ 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)
I have created a review for the requested 'llvm-dwarfdump' updates (the printing part) in https://reviews.llvm.org/D58442
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.
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).
Feb 19 2019
Feb 18 2019
- 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
Feb 15 2019
Addressed the inlined location expressions.
Feb 13 2019
NVPTX supports only DWARF2 and does not know anything about DWARF5 operations. Also, it does not support any type of the expression in the DWARF sections, except for <section_name>+-<int_offset>.
I see now that the inlined places i.e. these:
git grep addBlock.*DW_AT_location lib/CodeGen/AsmPrinter lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp: addBlock(*VariableDIE, dwarf::DW_AT_location, DwarfExpr->finalize()); lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp: addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize()); lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp: addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize()); lib/CodeGen/AsmPrinter/DwarfUnit.cpp: addBlock(ParamDIE, dwarf::DW_AT_location, Loc);
work quite differently and that will need some additional work.
Feb 12 2019
Removed Label from DIE (added DenseMap to CU and pass it around). Rebased to top of trunk.
Feb 11 2019
Addresses most of the open issues I had. The major question that remains is what to do when not targeting Dwarf v5.
Feb 7 2019
Updated to DW_OP_LLVM_convert as suggested, did the pretty printing and updated a bunch of llvm-lit tests to cope with the newly generated labels. Now the IR format looks like this:
!DIExpression(DW_OP_LLVM_convert, 16, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)
There are some open issues that I would like feedback on. I will add inline comments at those locations.
Feb 6 2019
All three of these operations expect a DW_TAG_basic_type as argument. So we could easily introduce a DW_OP_LLVM_basic_type <signedness> <bits> operation that gets expanded in AsmPrinter into DW_TAG_basic_types like you do now and thus avoid having to deal with TrackingMDRefs. This would also reduce the memory footprint of all DIExpressions that don't need a type argument.
Dropping all the metadata stuff and specifying the bit size and type encoding directly in the expression vector. e.g.
!DIExpression(DW_OP_convert, 8, 5, DW_OP_convert, 32, 5, DW_OP_stack_value)
This approach certainly has its advantages and require far less changes to existing code.
Feb 5 2019
Dwarf5 DW_OP_convert support.
If you choose this path, one way to solve the issue of what to do with the dangling types that are produced by the conversion operations would be to create them in a separate DICompileUnit with a compiler-generated name and location. This way the types should get uniqued automatically during LTO, and you don't need to worry about llvm-link & friends.
Feb 4 2019
the type reference should be a metadata operand. Otherwise you'll need to implement support in llvm-link etc, too.
I would just implement the type reference as normal metadata operands in DIExpression and bake the knowledge that DW_OP_convert consumes one of the metadata operands into the DIExpression operand iterator.
Feb 1 2019
Time to switch the approach. This time try adding support for typed Dwarf5 stack ops such as DW_OP_convert.
Jan 28 2019
Insert DW_OP_deref_size in Prologue Epilogue Inserter.
Jan 24 2019
Jan 21 2019
With the fragment approach we still have problems in e.g. lib/CodeGen/PrologEpilogInserter.cpp:1105 where the stack location of a function argument is prepended to the expression and a sized DW_OP_deref would need to be inserted.
Jan 17 2019
Uploading a prototype implementation using the DW_OP_LLVM_fragment as @bjope described. There are still some issues that need to be worked out with this so it is not final but rather I would like some input on if going down this track would be considered an acceptable solution. If there are clear objections to this approach it would be good to hear them early.
Jan 15 2019
Jan 14 2019
Unfortunately I think that there are additional complications in that our sext/zext computations will have to also depend on the endianness of the target and if the value is stored in memory or not (as having the value loaded as the 'wrong type' would require bytes to be swapped on a big endian target before it could be sext/zext by this expression).
Jan 13 2019
Jan 11 2019
Jan 3 2019
Jan 2 2019
Jan 1 2019
Dec 27 2018
Dec 21 2018
Dec 7 2018
Dec 6 2018
Dec 5 2018
Did some more experiments, trying to clean up the code and make it more generic.
Instead of only determining if X != 0 (which is clearly not sufficient) try to produce a value range for the variable and use that.
Nov 30 2018
Thanks for reviewing :)