This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Stub support for PPC
ClosedPublic

Authored by davide on Jan 7 2016, 1:47 PM.

Diff Detail

Event Timeline

davide updated this revision to Diff 44253.Jan 7 2016, 1:47 PM
davide retitled this revision from to [ELF] Stub support for PPC.
davide updated this object.
davide added reviewers: ruiu, rafael, hfinkel.
davide set the repository for this revision to rL LLVM.
davide added subscribers: jhibbits, emaste.
ruiu added inline comments.Jan 7 2016, 1:53 PM
ELF/Target.cpp
966

Can you fill this function to handle some basic relocations, so that it can handle some basic programs (such as a hello world written in assembly)? You can create another patch which depends on this if you want to separate them. The point is that I don't want to leave this stub empty for a long period of time.

davide added a comment.Jan 7 2016, 1:56 PM

SS

ELF/Target.cpp
966

Sure thing. As already mentioned I'm not really PPC savvy so it might take a little bit (a day) before I study the assembler and come up with a non-trivial example. @hfinkel, do you have any recommendations?

davide updated this revision to Diff 44355.Jan 8 2016, 11:52 AM
davide removed rL LLVM as the repository for this revision.

The code now handles basic relocations correctly (enough to link an ASM hello world, as proposed by Rui).

davide marked an inline comment as done.Jan 8 2016, 11:53 AM

Addressed Rui's comment(s).

ruiu accepted this revision.Jan 8 2016, 12:14 PM
ruiu edited edge metadata.

LGTM with a few nits.

ELF/Target.cpp
964

Add a blank line before this line.

test/ELF/basicppc.s
2

Rename basicppc.s -> basic-ppc.s

test/ELF/ppc-relocs.s
4 ↗(On Diff #44355)

Above file has "REQUIRES: ppc" and here we have "REQUIRES: powerpc". Which is correct? Even if both are correct, please choose one of them and use it consistently.

This revision is now accepted and ready to land.Jan 8 2016, 12:14 PM
This revision was automatically updated to reflect the committed changes.