Page MenuHomePhabricator

[mips][mc][clang] Use pc-relative relocations in .eh_frame
Needs RevisionPublic

Authored by atanasyan on May 21 2020, 11:13 AM.

Details

Summary

Use pc-relative relocations in .eh_frame. R_MIPS_PC32 relocation is generated in 32-bit case. R_MIPS_PC32 | R_MIPS_64 relocation chain is generated in 64-bit case for all code model.

With this change object files can be linked by LLD without having to pass -Wl,-z,notext options.

For compatibility with tools unsupported 64-bit pc-relative relocations the patch introduces new command line options in Clang: mmips-pc64-rel and mno-mips-pc64-rel. These options passed to LLVM by Clang driver as -mmips-pc64-rel={true|false}.

Diff Detail

Event Timeline

atanasyan created this revision.May 21 2020, 11:13 AM
Herald added a project: Restricted Project. · View Herald Transcript

I cannot select default behaviour for the LLVM in case of generating .eh_frame sections for MIPS:

  1. Unconditionally use pc-relative relocations.
  2. Use pc-relative relocations by default, but provide options for switching to absolute relocations.
  3. Use absolute relocations by default, but provide options for switching to pc-relative relocations.

I prefer the second variant. Any comments?

Hiya,

I'd probably also go for 2 also as the default, as long as its configurable then just a sensible default is good

Thanks

FYI, I backported this and the other diff to llvm 10 and its all working nicely, now building all packages with lld without issue

Thanks!

MaskRay accepted this revision.Jul 28 2020, 7:57 AM

LGTM.

This revision is now accepted and ready to land.Jul 28 2020, 7:57 AM
arichardson accepted this revision.Nov 4 2020, 12:32 PM

Hi guys,

any chance of this making it into 11.1.0 or 12?

Hi,

Hi guys,

any chance of this making it into 11.1.0 or 12?

This task was out of my scope for a while. As far as I remember there were some problems with test cases. One more thing - I couldn't figure out how to enable/disable this functionality by clang command line options. MCObjectFileInfo and TargetLoweringObjectFileELF classes are far away from any target related context/settings. I do not think that this change (especially without any "disable" functionality) is a good candidate for 11.1.0 or 12 releases. But I will try to push it to the main branch soon. I'm sorry for the delay.

No worries - whenever you have time!

atanasyan updated this revision to Diff 367472.Thu, Aug 19, 5:48 AM
atanasyan retitled this revision from [WIP][mips] Use pc-relative relocations in .eh_frame to [mips][mc][clang] Use pc-relative relocations in .eh_frame.
atanasyan edited the summary of this revision. (Show Details)
atanasyan edited reviewers, added: emaste, grosbach; removed: sdardis, dsanders, espindola.
atanasyan added a project: Restricted Project.

For compatibility with tools unsupported 64-bit pc-relative relocations the patch introduces new command line options in Clang: mmips-pc64-rel and mno-mips-pc64-rel. These options passed to LLVM by Clang driver as -mmips-pc64-rel={true|false}. I could not find any better way to pass the option into MC layer.

MaskRay requested changes to this revision.Thu, Aug 19, 11:35 AM

MCTargetOptionsCommandFlags.cpp may be a better place for the internal cl::opt option.

llvm/lib/MC/MCObjectFileInfo.cpp
27

-m prefix is for assembler and dropver options.

Internal codegen options don't need the prefix.

This revision now requires changes to proceed.Thu, Aug 19, 11:35 AM

If you want to do this proper, you may look at how -fbinutils-version= was implemented.