[PATCH] D31028: [ELF] - Detemplate PltSection<ELFT> section.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 17 04:15:22 PDT 2017


grimar added a comment.

In https://reviews.llvm.org/D31028#703332, @ruiu wrote:

> This doesn't seem like an improvement. The new code look more complicated than the original one. Maybe you want to make these classes non-template classes, while keeping some member functions template?


I think it is.

I tried keeping single template method for PltSection in https://reviews.llvm.org/D30982. It also works for what I want to do finally, so committed it as https://reviews.llvm.org/rL298065;
But this patch not more complicated in my opinion. See original code:

  In<ELFT>::Plt->addEntry<ELFT>(Body);
  In<ELFT>::GotPlt->addEntry(Body);
  In<ELFT>::RelaPlt->addReloc({Target->PltRel, In<ELFT>::GotPlt,
                               Body.getGotPltOffset(), !Preemptible,
                               &Body, 0});

Does it clear that Plt->addEntry depends on RelaPlt->addReloc ? If you switch their order code will be broken, because
Plt->addEntry inside refers to offset of relocation in RelaPlt. 
Change made in this patch reveals this dependency making it explicit. I think that is itself simplifies logic.


https://reviews.llvm.org/D31028





More information about the llvm-commits mailing list