This is an archive of the discontinued LLVM Phabricator instance.

ELF/AMDGPU: Add support for R_AMDGPU_REL32 relocations
ClosedPublic

Authored by tstellarAMD on Jun 13 2016, 9:03 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellarAMD retitled this revision from to ELF/AMDGPU: Add support for R_AMDGPU_ABS32 relocations.
tstellarAMD updated this object.
tstellarAMD added reviewers: rafael, ruiu.
tstellarAMD added a subscriber: llvm-commits.
ruiu added inline comments.Jun 13 2016, 9:49 AM
ELF/Target.cpp
1435–1440 ↗(On Diff #60540)

Then you can make this to

assert(Type == R_AMDGPU_REL32);
write32le(Loc, Val);
1444 ↗(On Diff #60540)

This runs before relocateOne, so it is better to do error checking here rather than later.

if (Type == R_AMDGPU_REL32)
  return R_PC;
error("do not know how to handle relocation");
ruiu edited edge metadata.Jun 13 2016, 9:58 AM

Please update the commit message -- this handles not ABS32 but REL32.

rafael added inline comments.Jun 13 2016, 10:52 AM
test/ELF/amdgpu-relocs.s
11 ↗(On Diff #60540)

Do you really need all this just to generate a relocation?

tstellarAMD edited edge metadata.

Add better error handling.

tstellarAMD retitled this revision from ELF/AMDGPU: Add support for R_AMDGPU_ABS32 relocations to ELF/AMDGPU: Add support for R_AMDGPU_REL32 relocations.Jun 14 2016, 8:23 PM
tstellarAMD marked 2 inline comments as done.
tstellarAMD added inline comments.
ELF/Target.cpp
1440–1442 ↗(On Diff #60787)

I made a slight modification to your suggestion to avoid "no return value" warnings for getRelExpr().

ruiu accepted this revision.Jun 15 2016, 10:42 AM
ruiu edited edge metadata.

LGTM. Please address Rafael's comment before committing.

This revision is now accepted and ready to land.Jun 15 2016, 10:42 AM
This revision was automatically updated to reflect the committed changes.