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

Adhemerval Zanella via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 25 07:08:43 PDT 2016

zatrazz added inline comments.

Comment at: ELF/OutputSections.cpp:293
@@ +292,3 @@
+  // Tail here is amount of such special relocations.
+  Relocs.insert(Relocs.end() - Tail, Reloc);
emaste wrote:
> grimar wrote:
> > emaste wrote:
> > > zatrazz wrote:
> > > > emaste wrote:
> > > > > I wonder if there's a nicer way to handle this?
> > > > I though about this, but I think to remove the Tail it would require to maintain two or move relocation vectors (one for normal and another for tlsdesc) and this will just add more complexity IMHO.
> > > > 
> > > > However we might change in later if this shows to be a performance issue.
> > > > However we might change in later if this shows to be a performance issue.
> > > 
> > > To me having two vectors seems more clear. Anyway, we can certainly try it both ways alter on and evaluate the performance difference.
> > Problem here is that 2 vectors are not really a solution. I mean for this place we just need to put these relocations after all others, but actually there are other similar group of relocations also: IRelative, which are also should be placed at tail. If we one day we will want to implement them (I mean dynamic case, for static one we already did it and static case  does not have that problem), then 2 vectors probably is not the best design as we will need 3 of them then ?
> > 
> > At the same time, probably, to have two vectors for now would be more clear, I just kept in mind that all stuff above when implemented that in my version of patch and decided to have one vector basing on that thoughts.
> I tried a quick experiment with a std::vector<int>. First case called push_back 10 times, then `insert(v.end() - 10, i)`
Indeed 2 vectors is not a definitely solution, but I think we can use current approach now and think about a more flexible solution later (since now only aarch64 requires tlsdesc support in this layout).

I think a better solution would be to add a variable number of vectors with the addTls* function to add on the specific one (for instance 'jump slot', 'tlsdesc', and 'irelative'). It could be a DenseMap or similar associating an type to a vector. The Target will then provide the desirable layout for the got.plt and the class will write down as specified. What do you think?



More information about the llvm-commits mailing list