[PATCH] D27415: [ELF] - Replace MergeOutputSection with synthetic input section MergeSection.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 27 08:29:59 PST 2017


grimar added inline comments.


================
Comment at: ELF/OutputSections.cpp:134
+  if (auto *Sec = dyn_cast<MergeSyntheticSection<ELFT>>(C))
+    Sec->setOutputSection(this);
+
----------------
ruiu wrote:
> Do you really need this? This looks odd because you are handling MergeInputSection in a special way. Generally you want to avoid that as possible. If you really need this, this may be a sign that we need to fix something else.
When symbols are created they refer to MergeInputSection and not to synthetic merge section, created later.
I think that is fine. That piece of code simplified things a bit, though I was able to get rid of.


================
Comment at: ELF/Writer.cpp:200
   createSyntheticSections();
+  combineMergableSections(Symtab<ELFT>::X->Sections);
 
----------------
ruiu wrote:
> As you know we usually do not pass around `Symtab<ELFT>::X->Sections`.
Fixed.


https://reviews.llvm.org/D27415





More information about the llvm-commits mailing list