[PATCH] D16201: [ELF/AArch64] - Implemented set of R_AARCH64_TLSDESC_* relocations.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 19 00:39:03 PDT 2016

grimar added inline comments.

Comment at: ELF/OutputSections.cpp:289
@@ +288,3 @@
+  Relocs.insert(Relocs.end() - Tail, Reloc);
zatrazz wrote:
> grimar wrote:
> > Ah, Relocs is a vector, not list. That is bad line then. I`ll replace it with the list on next iteration.
> I do not think this is incorrect since the vector will rearrange the itens by expanding it internally. It might however show performance regression, although I also think changing it to std::list or similar will show less performance and more memory usage (even with itens  rearrange by inserting in the middle). If you check this small performance test (http://pastie.org/10764242), std::list append shows pretty much the same performance of the std::vector rearrange (the 13000000 is number of relocation in NewLLD doc).
Well, that test uses list of ints, but we have Relocs which is a struct with several fields:

template <class ELFT> struct DynamicReloc {
  typedef typename ELFT::uint uintX_t;
  uint32_t Type;
  SymbolBody *Sym = nullptr;
  InputSectionBase<ELFT> *OffsetSec = nullptr;
  uintX_t OffsetInSec = 0;
  bool UseSymVA = false;
  uintX_t Addend = 0;
So shift of one element should cost a bit more.
Perfomance probably will depend a lot on amount of that tail relocations what may vary from application to application. 

I don't think we should care much about memory usage here, there are probably not much relocations anyways, every list item should add only 2 pointers for each element, while vector anyways will reserve some unused place too.

I will leave that place of code as is for now.


More information about the llvm-commits mailing list