This is an archive of the discontinued LLVM Phabricator instance.

[ELF][MIPS] Pass InputFile and SymbolBody to the Target::relocateOne method
AbandonedPublic

Authored by atanasyan on Feb 2 2016, 11:33 AM.

Details

Reviewers
ruiu
rafael
Summary

The patch adds two new arguments to the Target::relocateOne routine and passes pointers to InputFile and SymbolBody. For all targets except MIPS at least for now it just increases amount of useless relocateOne's arguments. For MIPS it allows to cleanup InputSectionBase::relocate code and remove almost all if (EMachine == EM_MIPS) statements from it.

I have an idea how to remove the last if (EMachine == EM_MIPS) statement from the InputSectionBase::relocate, but it is for a separate patch.

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan updated this revision to Diff 46684.Feb 2 2016, 11:33 AM
atanasyan retitled this revision from to [ELF][MIPS] Pass InputFile and SymbolBody to the Target::relocateOne method.
atanasyan updated this object.
atanasyan added reviewers: rafael, ruiu.
atanasyan set the repository for this revision to rL LLVM.
atanasyan added a project: lld.
atanasyan added a subscriber: llvm-commits.
ruiu edited edge metadata.Feb 2 2016, 12:12 PM

I'm not sure if this is a good change. Moving MIPS-specific code inside MipsTargetInfo is generally a good thing, but this adds unused parameters to all archs including non-MIPS ones. So MIPS-specific stuff is leaking to other archs here.

If MIPS is so different than the other archs, how about creating relocateOneMips? Do you think that will simplify the code?

In D16817#342067, @ruiu wrote:

If MIPS is so different than the other archs, how about creating relocateOneMips? Do you think that will simplify the code?

Now we call the relocateOne in four places in the InputSectionBase::relocate() code. If we introduce relocateOneMips we have to add if (EMachine == EM_MIPS) statement to all these places.

if (Config->EMachine == EM_MIPS)
  Target->relocateOneMips(File, Body, BufLoc, BufEnd, Type, AddrLoc, SymVA + A, Size + A,
                          findMipsPairedReloc(Buf, SymIndex, Type, NextRelocs));
else
  Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc, SymVA + A, Size + A,
                      findMipsPairedReloc(Buf, SymIndex, Type, NextRelocs));

Not sure that this will look better.

From the other side, the R_MIPS_26 relocation uses different formulas to calculate result for local / global symbols. To implement that I will need to know the type of symbols in the relocateOne or will have to add more MIPS specific code to the InputSectionBase::relocate().

ruiu added a subscriber: ruiu.Feb 3 2016, 4:37 PM

I was experimenting a few ideas to apply relocations in different ways, but
didn't get a better way yet. It is possible that we will end up just having
to add two more parameters to the function, but I'd like to experiment a
little bit more. Please give me a few more days.

In D16817#343645, @ruiu wrote:

I was experimenting a few ideas to apply relocations in different ways, but
didn't get a better way yet. It is possible that we will end up just having
to add two more parameters to the function, but I'd like to experiment a
little bit more. Please give me a few more days.

Ok. I'm going to experiment with this code too. Thanks for your help.

atanasyan abandoned this revision.Feb 29 2016, 1:17 PM