Page MenuHomePhabricator

[RuntimeDyld] Adapt PPC64 relocations to PPC32
ClosedPublic

Authored by pasaulais on May 25 2015, 10:47 AM.

Details

Reviewers
hfinkel
lhames
Summary

RuntimeDyldELF implements many PPC64 relocations but none for PPC32. This patch adapts (copies, really) from PPC64 the code for two relocations that can be used to access global variables like in the attached test case.

Diff Detail

Repository
rL LLVM

Event Timeline

pasaulais updated this revision to Diff 26461.May 25 2015, 10:47 AM
pasaulais retitled this revision from to [RuntimeDyld] Adapt PPC64 relocations to PPC32.
pasaulais updated this object.
pasaulais edited the test plan for this revision. (Show Details)
pasaulais added reviewers: hfinkel, lhames.
pasaulais set the repository for this revision to rL LLVM.
pasaulais added a subscriber: Unknown Object (MLST).
hfinkel edited edge metadata.May 27 2015, 11:54 AM

I don't know how the JIT is tested for the PPC target. There only seem to be X86 and ARM tests in test/ExecutionEngine/RuntimeDyld

That's not good. The generic MCJIT tests are run on PPC64, but otherwise I don't know.

Can you please add a test for this patch (in test/ExecutionEngine/RuntimeDyld sounds good if that's easiest) -- (just remember that the new directory needs a lit.local.cfg file, like the others, that tests for 'PowerPC').

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
907

This can be isInt<32>(delta).

pasaulais updated this revision to Diff 26784.May 29 2015, 8:58 AM
pasaulais edited edge metadata.

Thanks for looking at the patch.

I have added a llvm-rtdyld testcase for 'R_PPC_ADDR16_LO' and 'R_PPC_ADDR16_HA' relocation types. It seems to work on my VM by running the commands manually. I haven't got around to test it with 'lit' yet however, as I currently cross-compile LLVM outside of the VM.

I also removed the code for 'R_PPC_REL32' since it is not exercised by the test case. It will be part of another patch once I write a test case for it.

PPC eh.... Interesting :D

Marcello

hfinkel added inline comments.Jun 1 2015, 5:37 AM
test/ExecutionEngine/RuntimeDyld/PowerPC/ppc32_elf_rel_addr16.ll
1 ↗(On Diff #26784)

Why is this file here? If this is just the file that was used to create the initial .s file, don't add it.

pasaulais updated this revision to Diff 29566.Jul 13 2015, 7:18 AM

Hi Hal,

Sorry for the long hiatus. I have removed the .ll file that was used to generate the assembyl file, and ran the test on a PPC VM:

  • Testing: 1 tests, 1 threads --

PASS: LLVM :: ExecutionEngine/RuntimeDyld/PowerPC/ppc32_elf_rel_addr16.s (1 of 1)
Testing Time: 0.42s

Expected Passes    : 1

Do you see any other issue with the patch?

Thanks,
Pierre-Andre

hfinkel accepted this revision.Jul 26 2015, 6:35 AM
hfinkel edited edge metadata.

LGTM, thanks!

This revision is now accepted and ready to land.Jul 26 2015, 6:35 AM

Thanks, Hal. As I don't have commit access, could you commit it for me please?

hfinkel closed this revision.Aug 4 2015, 8:30 AM

r243991