<div dir="ltr">Thank you for sharing it to me. It's quite interesting. Let me do it myself too. But in any way this patch is for correctness. Without this patch you can't create >4Gb executables on Windows (even on Win64.)</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 24, 2016 at 9:56 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This is the screenshot that I remember taking: <a href="http://reviews.llvm.org/F1983334" target="_blank">http://reviews.llvm.org/F1983334</a><div><br></div><div>Percentages are out of the overall runtime.</div><span class="HOEnZb"><font color="#888888"><div><br></div></font></span><div><span class="HOEnZb"><font color="#888888">-- Sean Silva</font></span><div><div class="h5"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 24, 2016 at 9:42 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">It is I think actually spent in StringTableBuilder. More specifically, StringTableBuilder spends time on inserting given strings (all pieces of splittable sections) into a hash table in order to uniquify them.</div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 24, 2016 at 9:39 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Be very careful with this code (e.g. changing data layouts). We spend over 20% of non-LTO time in it last time I looked (mostly the call to lower_bound IIRC).<span><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, May 21, 2016 at 5:13 PM, Rui Ueyama via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Author: ruiu<br>
Date: Sat May 21 19:13:04 2016<br>
New Revision: 270340<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=270340&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=270340&view=rev</a><br>
Log:<br>
Define SectionPiece and use it instead of std::pair<uint_t, uint_t>.<br>
<br>
We were using std::pair to represents pieces of splittable section<br>
contents. It hurt readability because "first" and "second" are not<br>
meaningful. This patch give them names.<br>
<br>
One more thing is that piecewise liveness information is stored to<br>
the second element of the pair as a special value of output section<br>
offset. It was confusing, so I defiend a new bit, "Live", in the<br>
new struct.<br>
<br>
Modified:<br>
    lld/trunk/ELF/InputSection.cpp<br>
    lld/trunk/ELF/InputSection.h<br>
    lld/trunk/ELF/MarkLive.cpp<br>
    lld/trunk/ELF/OutputSections.cpp<br>
    lld/trunk/ELF/Writer.cpp<br>
<br>
Modified: lld/trunk/ELF/InputSection.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=270340&r1=270339&r2=270340&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=270340&r1=270339&r2=270340&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/InputSection.cpp (original)<br>
+++ lld/trunk/ELF/InputSection.cpp Sat May 21 19:13:04 2016<br>
@@ -414,13 +414,12 @@ typename ELFT::uint EHInputSection<ELFT><br>
   // identify the start of the output .eh_frame. Handle this special case.<br>
   if (this->getSectionHdr()->sh_size == 0)<br>
     return Offset;<br>
-  std::pair<uintX_t, uintX_t> *I = this->getRangeAndSize(Offset).first;<br>
-  uintX_t Base = I->second;<br>
-  if (Base == uintX_t(-1))<br>
+  SectionPiece *Piece = this->getRangeAndSize(Offset).first;<br>
+  if (Piece->OutputOff == size_t(-1))<br>
     return -1; // Not in the output<br>
<br>
-  uintX_t Addend = Offset - I->first;<br>
-  return Base + Addend;<br>
+  uintX_t Addend = Offset - Piece->InputOff;<br>
+  return Piece->OutputOff + Addend;<br>
 }<br>
<br>
 static size_t findNull(StringRef S, size_t EntSize) {<br>
@@ -443,17 +442,14 @@ MergeInputSection<ELFT>::MergeInputSecti<br>
   uintX_t EntSize = Header->sh_entsize;<br>
   ArrayRef<uint8_t> D = this->getSectionData();<br>
   StringRef Data((const char *)D.data(), D.size());<br>
-  std::vector<std::pair<uintX_t, uintX_t>> &Offsets = this->Offsets;<br>
<br>
-  uintX_t V = Config->GcSections ? MergeInputSection<ELFT>::PieceDead<br>
-                                 : MergeInputSection<ELFT>::PieceLive;<br>
   if (Header->sh_flags & SHF_STRINGS) {<br>
     uintX_t Offset = 0;<br>
     while (!Data.empty()) {<br>
       size_t End = findNull(Data, EntSize);<br>
       if (End == StringRef::npos)<br>
         fatal("string is not null terminated");<br>
-      Offsets.push_back(std::make_pair(Offset, V));<br>
+      this->Pieces.emplace_back(Offset);<br>
       uintX_t Size = End + EntSize;<br>
       Data = Data.substr(Size);<br>
       Offset += Size;<br>
@@ -465,7 +461,7 @@ MergeInputSection<ELFT>::MergeInputSecti<br>
   size_t Size = Data.size();<br>
   assert((Size % EntSize) == 0);<br>
   for (unsigned I = 0, N = Size; I != N; I += EntSize)<br>
-    Offsets.push_back(std::make_pair(I, V));<br>
+    this->Pieces.emplace_back(I);<br>
 }<br>
<br>
 template <class ELFT><br>
@@ -474,8 +470,7 @@ bool MergeInputSection<ELFT>::classof(co<br>
 }<br>
<br>
 template <class ELFT><br>
-std::pair<std::pair<typename ELFT::uint, typename ELFT::uint> *,<br>
-          typename ELFT::uint><br>
+std::pair<SectionPiece *, typename ELFT::uint><br>
 SplitInputSection<ELFT>::getRangeAndSize(uintX_t Offset) {<br>
   ArrayRef<uint8_t> D = this->getSectionData();<br>
   StringRef Data((const char *)D.data(), D.size());<br>
@@ -485,37 +480,32 @@ SplitInputSection<ELFT>::getRangeAndSize<br>
<br>
   // Find the element this offset points to.<br>
   auto I = std::upper_bound(<br>
-      Offsets.begin(), Offsets.end(), Offset,<br>
-      [](const uintX_t &A, const std::pair<uintX_t, uintX_t> &B) {<br>
-        return A < B.first;<br>
-      });<br>
-  uintX_t End = I == Offsets.end() ? Data.size() : I->first;<br>
+      Pieces.begin(), Pieces.end(), Offset,<br>
+      [](const uintX_t &A, const SectionPiece &B) { return A < B.InputOff; });<br>
+  uintX_t End = (I == Pieces.end()) ? Data.size() : I->InputOff;<br>
   --I;<br>
-  return std::make_pair(&*I, End);<br>
+  return {&*I, End};<br>
 }<br>
<br>
 template <class ELFT><br>
 typename ELFT::uint MergeInputSection<ELFT>::getOffset(uintX_t Offset) {<br>
-  std::pair<std::pair<uintX_t, uintX_t> *, uintX_t> T =<br>
-      this->getRangeAndSize(Offset);<br>
-  std::pair<uintX_t, uintX_t> *I = T.first;<br>
+  std::pair<SectionPiece *, uintX_t> T = this->getRangeAndSize(Offset);<br>
+  SectionPiece &Piece = *T.first;<br>
   uintX_t End = T.second;<br>
-  uintX_t Start = I->first;<br>
+  assert(Piece.Live);<br>
<br>
   // Compute the Addend and if the Base is cached, return.<br>
-  uintX_t Addend = Offset - Start;<br>
-  uintX_t &Base = I->second;<br>
-  assert(Base != MergeInputSection<ELFT>::PieceDead);<br>
-  if (Base != MergeInputSection<ELFT>::PieceLive)<br>
-    return Base + Addend;<br>
+  uintX_t Addend = Offset - Piece.InputOff;<br>
+  if (Piece.OutputOff != size_t(-1))<br>
+    return Piece.OutputOff + Addend;<br>
<br>
   // Map the base to the offset in the output section and cache it.<br>
   ArrayRef<uint8_t> D = this->getSectionData();<br>
   StringRef Data((const char *)D.data(), D.size());<br>
-  StringRef Entry = Data.substr(Start, End - Start);<br>
+  StringRef Entry = Data.substr(Piece.InputOff, End - Piece.InputOff);<br>
   auto *MOS = static_cast<MergeOutputSection<ELFT> *>(this->OutSec);<br>
-  Base = MOS->getOffset(Entry);<br>
-  return Base + Addend;<br>
+  Piece.OutputOff = MOS->getOffset(Entry);<br>
+  return Piece.OutputOff + Addend;<br>
 }<br>
<br>
 template <class ELFT><br>
<br>
Modified: lld/trunk/ELF/InputSection.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.h?rev=270340&r1=270339&r2=270340&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.h?rev=270340&r1=270339&r2=270340&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/InputSection.h (original)<br>
+++ lld/trunk/ELF/InputSection.h Sat May 21 19:13:04 2016<br>
@@ -130,6 +130,14 @@ public:<br>
<br>
 template <class ELFT> InputSectionBase<ELFT> InputSectionBase<ELFT>::Discarded;<br>
<br>
+// SectionPiece represents a piece of splittable section contents.<br>
+struct SectionPiece {<br>
+  SectionPiece(size_t I) : InputOff(I), Live(!Config->GcSections) {}<br>
+  size_t InputOff;<br>
+  size_t OutputOff = -1;<br>
+  bool Live;<br>
+};<br>
+<br>
 // Usually sections are copied to the output as atomic chunks of data,<br>
 // but some special types of sections are split into small pieces of data<br>
 // and each piece is copied to a different place in the output.<br>
@@ -142,24 +150,11 @@ public:<br>
   SplitInputSection(ObjectFile<ELFT> *File, const Elf_Shdr *Header,<br>
                     typename InputSectionBase<ELFT>::Kind SectionKind);<br>
<br>
-  // For each piece of data, we maintain the offsets in the input section and<br>
-  // in the output section.<br>
-  std::vector<std::pair<uintX_t, uintX_t>> Offsets;<br>
-<br>
-  // Merge input sections may use the following special values as the output<br>
-  // section offset:<br>
-  enum {<br>
-    // The piece is dead.<br>
-    PieceDead = uintX_t(-1),<br>
-    // The piece is live, but an offset has not yet been assigned. After offsets<br>
-    // have been assigned, if the output section uses tail merging, the field<br>
-    // will still have this value and the output section offset is computed<br>
-    // lazilly.<br>
-    PieceLive = uintX_t(-2),<br>
-  };<br>
+  // Splittable sections are handled as a sequence of data<br>
+  // rather than a single large blob of data.<br>
+  std::vector<SectionPiece> Pieces;<br>
<br>
-  std::pair<std::pair<uintX_t, uintX_t> *, uintX_t><br>
-  getRangeAndSize(uintX_t Offset);<br>
+  std::pair<SectionPiece *, uintX_t> getRangeAndSize(uintX_t Offset);<br>
 };<br>
<br>
 // This corresponds to a SHF_MERGE section of an input file.<br>
<br>
Modified: lld/trunk/ELF/MarkLive.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/MarkLive.cpp?rev=270340&r1=270339&r2=270340&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/MarkLive.cpp?rev=270340&r1=270339&r2=270340&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/MarkLive.cpp (original)<br>
+++ lld/trunk/ELF/MarkLive.cpp Sat May 21 19:13:04 2016<br>
@@ -143,9 +143,8 @@ template <class ELFT> void elf::markLive<br>
     if (!R.Sec)<br>
       return;<br>
     if (auto *MS = dyn_cast<MergeInputSection<ELFT>>(R.Sec)) {<br>
-      std::pair<std::pair<uintX_t, uintX_t> *, uintX_t> T =<br>
-          MS->getRangeAndSize(R.Offset);<br>
-      T.first->second = MergeInputSection<ELFT>::PieceLive;<br>
+      std::pair<SectionPiece *, uintX_t> T = MS->getRangeAndSize(R.Offset);<br>
+      T.first->Live = true;<br>
     }<br>
     if (R.Sec->Live)<br>
       return;<br>
<br>
Modified: lld/trunk/ELF/OutputSections.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=270340&r1=270339&r2=270340&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=270340&r1=270339&r2=270340&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/OutputSections.cpp (original)<br>
+++ lld/trunk/ELF/OutputSections.cpp Sat May 21 19:13:04 2016<br>
@@ -978,10 +978,9 @@ EHRegion<ELFT>::EHRegion(EHInputSection<<br>
<br>
 template <class ELFT> StringRef EHRegion<ELFT>::data() const {<br>
   ArrayRef<uint8_t> SecData = S->getSectionData();<br>
-  ArrayRef<std::pair<uintX_t, uintX_t>> Offsets = S->Offsets;<br>
-  size_t Start = Offsets[Index].first;<br>
-  size_t End =<br>
-      Index == Offsets.size() - 1 ? SecData.size() : Offsets[Index + 1].first;<br>
+  size_t Start = S->Pieces[Index].InputOff;<br>
+  size_t End = (Index == S->Pieces.size() - 1) ? SecData.size()<br>
+                                               : S->Pieces[Index + 1].InputOff;<br>
   return StringRef((const char *)SecData.data() + Start, End - Start);<br>
 }<br>
<br>
@@ -1142,8 +1141,8 @@ void EHOutputSection<ELFT>::addSectionAu<br>
<br>
   DenseMap<uintX_t, uintX_t> OffsetToIndex;<br>
   while (!D.empty()) {<br>
-    unsigned Index = S->Offsets.size();<br>
-    S->Offsets.push_back(std::make_pair(Offset, -1));<br>
+    unsigned Index = S->Pieces.size();<br>
+    S->Pieces.emplace_back(Offset);<br>
<br>
     uintX_t Length = readEntryLength<ELFT>(D);<br>
     // If CIE/FDE data length is zero then Length is 4, this<br>
@@ -1227,11 +1226,11 @@ template <class ELFT> void EHOutputSecti<br>
<br>
   size_t Offset = 0;<br>
   for (const Cie<ELFT> &C : Cies) {<br>
-    C.S->Offsets[C.Index].second = Offset;<br>
+    C.S->Pieces[C.Index].OutputOff = Offset;<br>
     Offset += alignTo(C.data().size(), sizeof(uintX_t));<br>
<br>
     for (const EHRegion<ELFT> &F : C.Fdes) {<br>
-      F.S->Offsets[F.Index].second = Offset;<br>
+      F.S->Pieces[F.Index].OutputOff = Offset;<br>
       Offset += alignTo(F.data().size(), sizeof(uintX_t));<br>
     }<br>
   }<br>
@@ -1240,11 +1239,11 @@ template <class ELFT> void EHOutputSecti<br>
 template <class ELFT> void EHOutputSection<ELFT>::writeTo(uint8_t *Buf) {<br>
   const endianness E = ELFT::TargetEndianness;<br>
   for (const Cie<ELFT> &C : Cies) {<br>
-    size_t CieOffset = C.S->Offsets[C.Index].second;<br>
+    size_t CieOffset = C.S->Pieces[C.Index].OutputOff;<br>
     writeCieFde<ELFT>(Buf + CieOffset, C.data());<br>
<br>
     for (const EHRegion<ELFT> &F : C.Fdes) {<br>
-      size_t Offset = F.S->Offsets[F.Index].second;<br>
+      size_t Offset = F.S->Pieces[F.Index].OutputOff;<br>
       writeCieFde<ELFT>(Buf + Offset, F.data());<br>
       write32<E>(Buf + Offset + 4, Offset + 4 - CieOffset); // Pointer<br>
<br>
@@ -1284,32 +1283,30 @@ void MergeOutputSection<ELFT>::addSectio<br>
   StringRef Data((const char *)D.data(), D.size());<br>
   uintX_t EntSize = S->getSectionHdr()->sh_entsize;<br>
   this->Header.sh_entsize = EntSize;<br>
-  MutableArrayRef<std::pair<uintX_t, uintX_t>> Offsets = S->Offsets;<br>
<br>
   // If this is of type string, the contents are null-terminated strings.<br>
   if (this->Header.sh_flags & SHF_STRINGS) {<br>
-    for (unsigned I = 0, N = Offsets.size(); I != N; ++I) {<br>
-      auto &P = Offsets[I];<br>
-      if (P.second == MergeInputSection<ELFT>::PieceDead)<br>
+    for (unsigned I = 0, N = S->Pieces.size(); I != N; ++I) {<br>
+      SectionPiece &Piece = S->Pieces[I];<br>
+      if (!Piece.Live)<br>
         continue;<br>
<br>
-      uintX_t Start = P.first;<br>
-      uintX_t End = (I == N - 1) ? Data.size() : Offsets[I + 1].first;<br>
+      uintX_t Start = Piece.InputOff;<br>
+      uintX_t End = (I == N - 1) ? Data.size() : S->Pieces[I + 1].InputOff;<br>
       StringRef Entry = Data.substr(Start, End - Start);<br>
       uintX_t OutputOffset = Builder.add(Entry);<br>
-      if (shouldTailMerge())<br>
-        OutputOffset = MergeInputSection<ELFT>::PieceLive;<br>
-      P.second = OutputOffset;<br>
+      if (!shouldTailMerge())<br>
+        Piece.OutputOff = OutputOffset;<br>
     }<br>
     return;<br>
   }<br>
<br>
   // If this is not of type string, every entry has the same size.<br>
-  for (auto &P : Offsets) {<br>
-    if (P.second == (uintX_t)-1)<br>
+  for (SectionPiece &Piece : S->Pieces) {<br>
+    if (!Piece.Live)<br>
       continue;<br>
-    StringRef Entry = Data.substr(P.first, EntSize);<br>
-    P.second = Builder.add(Entry);<br>
+    StringRef Entry = Data.substr(Piece.InputOff, EntSize);<br>
+    Piece.OutputOff = Builder.add(Entry);<br>
   }<br>
 }<br>
<br>
<br>
Modified: lld/trunk/ELF/Writer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=270340&r1=270339&r2=270340&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=270340&r1=270339&r2=270340&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/Writer.cpp (original)<br>
+++ lld/trunk/ELF/Writer.cpp Sat May 21 19:13:04 2016<br>
@@ -855,8 +855,7 @@ template <class ELFT> static bool includ<br>
     if (!D->Section->Live)<br>
       return false;<br>
     if (auto *S = dyn_cast<MergeInputSection<ELFT>>(D->Section))<br>
-      if (S->getRangeAndSize(D->Value).first->second ==<br>
-          MergeInputSection<ELFT>::PieceDead)<br>
+      if (!S->getRangeAndSize(D->Value).first->Live)<br>
         return false;<br>
   }<br>
   return true;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div></div></div></div>
</blockquote></div><br></div>