Page MenuHomePhabricator
Differential D24722

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

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

Details

Reviewers
vkalintiris
zoran.jovanovic
seanbruno
Commits
rGc08af6db5b80: [mips] Correct jal expansion for local symbols in .local directives.
rL287918: [mips] Correct jal expansion for local symbols in .local directives.
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

Diff Detail

Repository
rL LLVM

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 reviewers: vkalintiris, zoran.jovanovic.
sdardis added subscribers: llvm-commits, dsanders, seanbruno.
Herald added a subscriber: sdardis. · View Herald TranscriptSep 19 2016, 5:23 AM
sdardis mentioned this in D24721: [MC][MCParser] Mark local and weak symbols as such when parsing assembly text.Sep 19 2016, 5:24 AM
sdardis added a parent revision: D24721: [MC][MCParser] Mark local and weak symbols as such when parsing assembly text.
sdardis mentioned this in D23497: [wip][mips] Correct jal expansion for local symbols in .local directives..
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
sdardis removed a parent revision: D24721: [MC][MCParser] Mark local and weak symbols as such when parsing assembly text.Nov 1 2016, 4:07 AM
seanbruno added a comment.Nov 21 2016, 6:57 AM

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

sdardis added a comment.Nov 23 2016, 1:30 AM

@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 added a comment.Nov 25 2016, 2:52 AM
In D24722#603980, @sdardis wrote:

@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.

Closed by commit rL287918: [mips] Correct jal expansion for local symbols in .local directives. (authored by sdardis). · Explain WhyNov 25 2016, 3:16 AM
This revision was automatically updated to reflect the committed changes.
sdardis added a comment.Nov 25 2016, 3:17 AM

Thanks for the review.

Revision Contents
Changeset List

PathSize
llvm/
trunk/
lib/
Target/
Mips/
AsmParser/
3 lines
test/
MC/
Mips/
155 lines

Diff 79290

View Options

llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp

Loading...
View Options

llvm/trunk/test/MC/Mips/expansion-jal-sym-pic.s

Loading...