[PATCH] D23768: [ELF] Linkerscript: eliminate LayoutInputSection

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 31 09:05:04 PDT 2016


ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:214-217
@@ -257,1 +213,6 @@
+    for (InputSectionBase<ELFT> *S : V)
+      if (S->OutSec == nullptr) {
+        Ret.push_back(S);
+        S->OutSec = reinterpret_cast<OutputSectionBase<ELFT> *>(-1ULL);
+      }
   }
----------------
evgeny777 wrote:
> ruiu wrote:
> > Setting -1 to the pointer to record whether an input section is added or not is an unintended use of the pointer and pretty confusing. Please use a DenseSet to uniquefy.
> This will cause growth of memory consumption and slightly slow down things. May be using some fake output section is better?
> 
> ```
> OutputSectionBase<ELFT> FakeSec;
> for (InputSectionBase<ELFT> *S : V)
>       if (!S->OutSec) {
>          Ret.push_back(S);
>          S->OutSec = &FakeSec;
>       }
> ```
That also has a prolonged side effect -- the assigned pointers will be visible from outside after returning from this function even though they are not valid pointers. That confuses readers as to why we needed to assign pointers. I was actually wondering. I don't think this is a performance critical path (linker scripts are not that fast anyways). So I'd use a map.


https://reviews.llvm.org/D23768





More information about the llvm-commits mailing list