This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/SI: Add support for R_AMDGPU_GOTPCREL
ClosedPublic

Authored by tstellarAMD on Jun 17 2016, 2:46 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellarAMD retitled this revision from to AMDGPU/SI: Add support for R_AMDGPU_GOTPCREL.
tstellarAMD updated this object.
tstellarAMD added a subscriber: llvm-commits.
arsenm added inline comments.Jun 17 2016, 2:58 PM
lib/Target/AMDGPU/SIISelLowering.cpp
1455 ↗(On Diff #61135)

Missing space after ==

1478–1483 ↗(On Diff #61135)

D13575 really needs to be committed

1487 ↗(On Diff #61135)

Should be able to make invariant = true. Shouldn't the alignment also be set?

tstellarAMD marked 2 inline comments as done.

Set alignment and isInvariant for load from the GOT.

lib/Target/AMDGPU/SIISelLowering.cpp
1478–1483 ↗(On Diff #61135)

I don't think D13575 will help with this. MachinePointerInfo assumes address space zero for all PseudoSourceValues.

arsenm added inline comments.Jun 17 2016, 10:19 PM
lib/Target/AMDGPU/SIISelLowering.cpp
1478–1483 ↗(On Diff #61157)

I think that's more of a problem with the MachinePointerInfo rather than PseudoSourceValue.

Actually there's already a GOT type. PseudoSourceValueManager needs an address space argument added through it.

Use a PseudoSourceValue to represent the GOT.

arsenm added inline comments.Jun 30 2016, 5:40 PM
lib/Target/AMDGPU/SIISelLowering.cpp
505 ↗(On Diff #61799)

Typo 'thatg'

1428–1434 ↗(On Diff #61799)

This looks like a broken merge

Address review comments and also stop using PseudoSourceValue for
the GOT address. I will add this back in in a follow up commit once
http://reviews.llvm.org/D21688 is committed.

arsenm added inline comments.Jul 8 2016, 3:21 PM
lib/Target/AMDGPU/SIISelLowering.cpp
30 ↗(On Diff #63330)

I don't think I see anything that uses anything new from this

lib/Target/AMDGPU/SIMachineFunctionInfo.h
19 ↗(On Diff #63330)

Belongs with pseudosourcevalue patch

399 ↗(On Diff #63330)

Extra line

Address review comments.

tstellarAMD marked 3 inline comments as done.Jul 11 2016, 3:35 AM
arsenm accepted this revision.Jul 12 2016, 11:00 AM
arsenm edited edge metadata.

LGTM with the one fix

lib/Target/AMDGPU/SIISelLowering.cpp
520 ↗(On Diff #63482)

You don't need the or_null because of the !Ptr check above

This revision is now accepted and ready to land.Jul 12 2016, 11:00 AM
kzhuravl accepted this revision.Jul 12 2016, 12:26 PM
kzhuravl edited edge metadata.

LGTM

This revision was automatically updated to reflect the committed changes.
llvm/trunk/test/CodeGen/AMDGPU/global-variable-relocs.ll