[LLVMdev] [lld] Relocation reading refactoring

[shankarke]

We could stick to option A and implement or move to option B need be. 

Sent from my iPhone

> On Mar 6, 2014, at 1:58, "Simon Atanasyan-2 [via LLVM]" <ml-node+s1065342n66564h67 at n5.nabble.com> wrote:
> 
> Hi Shankar, 
> 
> I almost implement ELFRelocationReader but still not completely sure 
> that this is a right direction. Suppose somebody wants to override 
> creation of the `ELFReference` object from the `Elf_Rela` or `Elf_Rel` 
> record. Let's consider two implementations A and B: 
> 
> A: 
> ===== 
> 1. Factor out `ELFReference` creation from 
> `createDefinedAtomAndAssignRelocations` into a couple of virtual 
> functions with the following signature: 
> 
> ELFReference<ELFT> * 
> createRelocationReference(const Elf_Sym &symbol, const Elf_Rela &rai); 
> 
> ELFReference<ELFT> * 
> createRelocationReference(const Elf_Sym &symbol, const Elf_Rel &ri, 
>                           ArrayRef<uint8_t> content); 
> 
> 2. Override one or both these methods in the <target name>ELFFile class. 
> 
> B: 
> ===== 
> 1. Create new class `ELFRelocationReader` and factor out 
> `ELFReference` creation into the methods of this class. 
> 2. Add new virtual function `createRelocationReader` to the `ELFFile` 
> to create an instance of the `ELFRelocationReader`. 
> 3. Subclass `ELFRelocationReader` and implement a target specific 
> relocation reading. 
> 4. Override `ELFFile::createRelocationReader` to create 
> `ELFRelocationReader` subclass. 
> 
> In both implementations we just move some portion of code from one 
> place to another. We do not need to reuse the relocation reading 
> functionality somewhere else. But the implementation "A" requires much 
> less modifications and does not introduce any new entities. So from my 
> point of view both approaches provide the same result but the 
> implementation "B" much more wordy. Will we get any benefits if 
> implement option "B"? 
> 
> Thanks. 
> 
> On Thu, Feb 27, 2014 at 2:33 AM, Shankar Easwaran 
> <[hidden email]> wrote: 
> > The subclasses for ELFFile would use it I thought. 
> > 
> > For example if a target wants the basic logic for converting an input file 
> > to atoms, the target could just override the relocation handler to get all 
> > the references. 
> 
> [...] 
> 
> > On 2/26/2014 3:16 PM, Simon Atanasyan wrote: 
> 
> [...] 
> 
> >> Who will be user of this class? If it is still only ELFFile class, 
> >> what benefits will we get from separation of this logic? 
> >> 
> >> template <class ELFT> class ELFRelocationReader { 
> >> public: 
> >>    ELFRelocationReader(.....); 
> >> 
> >>    // Returns all created references. 
> >>    ReferenceRangeT getAllReferences(); 
> >> 
> >>    // Returns references for specified section/symbol. 
> >>    ReferenceRangeT getReferences(StringRef sectionName, 
> >>                                  Elf_Sym *symbol, 
> >>                                  ArrayRef<uint8_t> content); 
> >> 
> >> protected: 
> >>    // Target can override these methods in the inherited class. 
> >>    virtual ELFReference<ELFT> *createReference(Elf_Rela &rel, Elf_Sym 
> >> *symbol); 
> >>    virtual ELFReference<ELFT> *createReference(Elf_Rel &rel, Elf_Sym 
> >> *symbol); 
> >> }; 
> >> 
> >> On Wed, Feb 26, 2014 at 9:42 PM, Shankar Easwaran 
> >> <[hidden email]> wrote: 
> >>> 
> >>> I was thinking of having a separate class that would return a vector of 
> >>> ELFReferences when you the reader looks at a section and symbol. 
> >>> 
> >>> The class would be constructed with 
> >>> _relocationAddendReferences/_relocationReferences. 
> >>> 
> >>> Each subtarget could make use of the functionality and create a different 
> >>> type of ELFReference on a need basis. 
> >> 
> >> [...] 
> >> 
> >>>> What do you mean by removing relocation reading from the 
> >>>> ELFObjectFile? I considered to customize the relocation reading for 
> >>>> MIPS targets and my first idea was to factor out ELFReference creation 
> >>>> into a couple of virtual functions. The first one is for Elf_Rel, the 
> >>>> second one is for Elf_Rela. Then I planned to override these functions 
> >>>> in the MipsELFFile class. But it looks like you have more profound 
> >>>> idea. Could you share it?
> 
> -- 
> Simon 
> _______________________________________________ 
> LLVM Developers mailing list 
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
> 
> If you reply to this email, your message will be added to the discussion below:
> http://llvm.1065342.n5.nabble.com/lld-Relocation-reading-refactoring-tp66310p66564.html
> To start a new topic under LLVM - Dev, email ml-node+s1065342n3h76 at n5.nabble.com 
> To unsubscribe from LLVM - Dev, click here.
> NAML




--
View this message in context: http://llvm.1065342.n5.nabble.com/lld-Relocation-reading-refactoring-tp66310p66568.html
Sent from the LLVM - Dev mailing list archive at Nabble.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140306/54f731a3/attachment.html>