- User Since
- Sep 6 2015, 10:51 PM (293 w, 1 d)
Fri, Apr 16
Mar 10 2021
Mar 7 2021
The fix resolves the issue on my environment. Thank you for the quick response!
Mar 5 2021
This change causes build failures in my Windows environment.
Feb 22 2021
Looks good, thanks!
Feb 19 2021
Why should a service class like MCStreamer define the architecture of its clients? It is just not its area of responsibility.
Maybe it'll make sense to do that in this patch. There will be fewer changes in the tests.
(Moving the thread, as it's more suitable here)
More in the sense that it limits the flexibility of calling code without any real profit. For example, the caller now cannot create the label in advance. For instance, Dwarf5AccelTableWriter::ContributionEnd was initialized when the object of that class was created. Simple and solid. Now the initialization is postponed until a method of another class, Dwarf5AccelTableWriter::Header is called. The tracking is a bit more challenging now.
Feb 17 2021
Feb 16 2021
Ping. What should be done to proceed with this?
Feb 15 2021
I guess we can simplify that even further by emitting the temporary symbol right after the "length of the prologue" field. In that case, PreHeaderLengthBytes will not be needed at all.
Thank you, @dblaikie!
Feb 12 2021
Feb 10 2021
The idea was to remove the Lo argument from MCStreamer::emitDwarfUnitLength(Hi, Lo, Comment) so that it becomes just MCStreamer::emitDwarfUnitLength(Hi, Comment). A temporary symbol can be created and emitted inside the method. And that should be a separate NFC patch.
Feb 9 2021
There is no need to create a new MCStreamer::emitDwarfLineUnitLength() method and make all the dependencies public. It is better to just use the already existed MCStreamer::emitDwarfUnitLength(). All you need to do is to create a temporary symbol and emit it right after calling emitDwarfUnitLength().
Feb 8 2021
There were so many ideas in the discussion that it looked like they not going to converge. The idea of waiting for v6 does not resolve the issue for existing standards, just postpones the possible solution for years with a yet unknown result. Adding new section names, flags, types, etc., might be promising at the first glance but requires updating lots of tools, including tools in different toolchains, which complicates achieving the result even further. That is why I feel it is necessary to illustrate my proposal with a complete patch, which is aimed to show that a simple, efficient, and standard-compliant solution is possible.
Feb 5 2021
Feb 4 2021
LGTM but please postpone committing it until the whole set of patches is completed so we can have a full image of the changes.
Feb 3 2021
Feb 2 2021
Seems OK. Something similar is in exception_object_alignment.pass.cpp.
It looks like my idea was not well-thought.
Feb 1 2021
Jan 29 2021
The comment was originally sent to D94668, but it looks like it suits here better.
Jan 15 2021
Is it possible to encapsulate the target-specific logic into some derived classes from MCStreamer so that MCStreamer itself and DWARF emitter classes work through a common interface and have no target-specific adjustments?
I suppose that you describe how .dwsect pseudo-op works. That is quite interesting, why they designed the feature to work that way. Is it recommended to reference debug sections through the label minus the length field size (4 or 12) or they provide some means to simplify the calculation? How an assembler output of their own compiler looks like?
Jan 14 2021
Trying to force CI to run.
- Updated as suggested. Thanks, @ldionne!
The current code violates the C++ standard, which says (in different sections, depending on the version):
Dec 20 2020
Nov 18 2020
Nov 16 2020
It looks like lld/test/COFF/lto-new-pass-manager.ll.obj was added to the patch by accident and should be removed.
Nov 13 2020
Well, this is a bit different from my original idea but is an overall good heuristic for many of the debug sections. It works for .debug_info, which is one of the biggest sections; it does not work for .debug_line, though, which is not that big as .debug_info, but potentially might become problematic in the (distant) future; it also does not work for .debug_abbrev, .debug_addr, .debug_ranges, and some others, which are usually not that big. However, the patch should be extended to support .debug_str, which can be even larger than .debug_info.
Nov 6 2020
Nov 5 2020
- The patch needs tests to check the added functionality.
- DWARF64 can be generated only for a limited number of targets. There should be diagnostics for invalid switch combinations to prevent misuse. @MaskRay mentioned that in the patch for llc, D87011#2254749, but that makes a lot more sense for user-level tools.
I suppose that it would be helpful to arrange debugging information sections so that DWARF64 comes after DWARF32, otherwise, some 32-bit relocations in the 32-bit info could not be resolved. But that idea might be a bit controversial because usually debugging information is expected to have the same order as the sections it refers to.
Nov 4 2020
To generate 64-bit debugging info, there should be enough to pass the switch through CLANG, right. Apart from that, we will probably need some compatibility checks so that using the switch in unsupported cases prints out diagnostics. There are also some improvements on the LLD side which are better to be done to support extremely large debugging information. Not all our tools fully support DWARF64 yet, etc.
Oct 28 2020
The switch is implemented only internally in LLVM. There is still some work to be done to enable producing 64-bit debugging info in clang, but I strayed a bit for another task. Hope to come back later this year.
Oct 27 2020
Oct 23 2020
Oct 22 2020
- Updated the test as suggested. Thanks, @bd1976llvm!
Oct 21 2020
Oct 19 2020
Oct 16 2020
Hmm, that starts looking like a discussion for the mailing list, but I guess that all interested people are already here and the conversation will be better preserved for further references if being linked to the review.
Oct 15 2020
Oct 14 2020
Gently ping. Is there anything I can improve in the fix so that it is accepted?
Oct 8 2020
I've made some tests and speculatively estimate the saving up to several seconds for a relatively large library (about 100000 symbols) which is referenced several hundred times. For me, that is noticeable.
Oct 6 2020
- Simplified the comment.
- Fixed the comment.
Sep 21 2020
Sep 16 2020
D87008 added creating a test AsmPrinter to the CodeGen unit tests, so it is now possible to fix the DIEHashTest.MemberBlock so that it provides an instance to the tested class.