This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Change how alignment is computed for argument lowering
ClosedPublic

Authored by wdng on Feb 16 2016, 3:39 PM.

Details

Summary

Fixes not treating i16 kernel arguments as 4-byte aligned when
added as legal.

Also fixes bug for mesa argument lowering. The implicit 36-byte offset
does not correctly align 64-bit arguments.

Diff Detail

Repository
rL LLVM

Event Timeline

arsenm updated this revision to Diff 48117.Feb 16 2016, 3:39 PM
arsenm retitled this revision from to AMDGPU: Change how alignment is computed for argument lowering.
arsenm updated this object.
arsenm added a reviewer: tstellarAMD.
arsenm added a subscriber: llvm-commits.
tstellarAMD accepted this revision.Feb 16 2016, 4:00 PM
tstellarAMD edited edge metadata.

LGTM.

lib/Target/AMDGPU/AMDGPUCallingConv.td
113 ↗(On Diff #48117)

Commented out code.

This revision is now accepted and ready to land.Feb 16 2016, 4:00 PM
wdng commandeered this revision.Jun 22 2016, 1:14 PM
wdng added a reviewer: arsenm.
wdng added a subscriber: wdng.

wdng updated this revision to Diff 61598.Jun 22 2016, 1:16 PM
wdng edited edge metadata.
wdng set the repository for this revision to rL LLVM.

Updated the patch based on Matt's previous patch.

arsenm added inline comments.Jun 29 2016, 12:39 PM
lib/Target/AMDGPU/SIISelLowering.cpp
565–566

You should be able to use one of the simpler getLoad operands that doesn't include the ISD::UNINDEXED

1512

This should be the same MinAlign(Subtarget->getKernargSegmentPtrAlignment(), Offset)

1600–1601

Should be the same MinAlign again

wdng updated this revision to Diff 62269.Jun 29 2016, 12:50 PM

Update diffs with full context.

wdng updated this revision to Diff 62941.Jul 6 2016, 12:45 PM
wdng marked 3 inline comments as done.

Changes based on Matt's comments.

wdng updated this revision to Diff 63135.Jul 7 2016, 2:16 PM

Using a simpler getLoad that no not use "ISD::UNINDEXED".

arsenm closed this revision.Apr 5 2020, 8:07 AM

Should be obsolete