This is an archive of the discontinued LLVM Phabricator instance.

[mips] Correct jal expansion for local symbols in .local directives.
ClosedPublic

Authored by sdardis on Sep 19 2016, 5:23 AM.

Details

Summary

This patch corrects the behaviour of code such as:

   .local foo
   jal foo
foo:

to use the correct jal expansion when writing ELF files.

Patch by: Daniel Sanders

Event Timeline

sdardis updated this revision to Diff 71814.Sep 19 2016, 5:23 AM
sdardis retitled this revision from to [mips] Correct jal expansion for local symbols in .local directives..
sdardis updated this object.
sdardis added subscribers: llvm-commits, dsanders, seanbruno.
seanbruno accepted this revision.Sep 28 2016, 3:24 PM
seanbruno added a reviewer: seanbruno.

Right, this seems to be required to link libc on FreeBSD. Thanks!

This revision is now accepted and ready to land.Sep 28 2016, 3:24 PM
seanbruno edited edge metadata.Sep 28 2016, 3:25 PM
seanbruno added a subscriber: emaste.
vkalintiris accepted this revision.Oct 19 2016, 4:47 AM
vkalintiris edited edge metadata.

LGTM once its dependency lands.

sdardis planned changes to this revision.Oct 19 2016, 4:59 AM

I'm going to revert this @dsanders 's old version, then make D24721 dependent on this. That cleans up the implementation of D24721, as the old version exposes that bug much clearer.

sdardis updated this revision to Diff 76539.Nov 1 2016, 3:56 AM
sdardis updated this object.
sdardis edited edge metadata.

Reverted this patch to Daniel's original version. The fix for the assembly case will be resolved once D24721 is committed.

This revision is now accepted and ready to land.Nov 1 2016, 3:56 AM

This is required to build libc on FreeBSD MIPS currently. Thanks for working on it!

@vkalintiris this patch ok without the dependency and FIXMEs? I should be able to get the other patch in as I can provide cleaner test cases.

@vkalintiris this patch ok without the dependency and FIXMEs? I should be able to get the other patch in as I can provide cleaner test cases.

Sounds good to me, go ahead and merge it.

This revision was automatically updated to reflect the committed changes.

Thanks for the review.