[PATCH] D24706: [ELF] - Partial support of --gdb-index command line option (Part 1).

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/OutputSections.cpp:81
@@ +80,3 @@
+  // and 5 more fields with different kinds of offsets.
+  CuListOffset = 6 * OffsetTypeSize;
+  CuTypesOffset = CuListOffset + CompilationUnits.size() * CompilationUnitSize;
----------------
ruiu wrote:
> According to the comment, `CuListOffset` can be a compile-time constant rather than computing at runtime here.
Done.

================
Comment at: ELF/OutputSections.cpp:114
@@ +113,3 @@
+  // Write the CU list.
+  for (std::pair<uintX_t, uintX_t>& CU : CompilationUnits) {
+    write64le(Buf, CU.first);
----------------
ruiu wrote:
> I think you can just make a copy here (i.e. remove `&`)
Done.

================
Comment at: ELF/SplitDebugInfo.cpp:10
@@ +9,3 @@
+//
+// File contains classes for implementation of --gdb-index command line option.
+//
----------------
ruiu wrote:
> aprantl wrote:
> > Any way this could be doxygen-ified?
> Part of this file comment was a copy from other document. You need to add citation.
As Rui mentioned, we do not use doxygen comments in lld at this moment.
Added quotes and references to source.

================
Comment at: ELF/SplitDebugInfo.cpp:17
@@ +16,3 @@
+//
+// Split debug information feature tries to solve the problems caused by huge
+// amounts of debug information in large applications.
----------------
aprantl wrote:
> Since LLVM already supports split debug info I don't think there is a need to re-explain this here — But there might be a better place to document this: Maybe it makes sense to move the general introuction to SourceLevelDebugging.rst?
Ok, I cut off most of information from this header and added just a very short intro for now.

================
Comment at: ELF/SplitDebugInfo.cpp:28
@@ +27,3 @@
+// needs them. More specific information can be found at
+// https://gcc.gnu.org/wiki/DebugFission.
+//
----------------
aprantl wrote:
> Maybe better link to the DWARF 5 specification?
> http://dwarfstd.org/Issues.php?type=closed4
Done.

================
Comment at: ELF/SplitDebugInfo.cpp:59
@@ +58,3 @@
+//
+// For costructing the .gdb_index section following steps should be performed:
+// 1) For file header nothing special should be done. It contains the offsets to
----------------
aprantl wrote:
> constructing
Fixed.


https://reviews.llvm.org/D24706