This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Use performOptimizedStructLayout for LDS sort
ClosedPublic

Authored by rampitec on Jun 15 2021, 4:06 PM.

Diff Detail

Event Timeline

rampitec created this revision.Jun 15 2021, 4:06 PM
rampitec requested review of this revision.Jun 15 2021, 4:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2021, 4:06 PM
Herald added a subscriber: wdng. · View Herald Transcript

PSDB passed.

hsmhsm accepted this revision.Jun 21 2021, 10:38 PM

LGTM apart from above minor comment.

llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
267

Now since we are calling performOptimizedStructLayout(), I am not sure, if the struct member with max align is guarenteed to be at offset 0.

This revision is now accepted and ready to land.Jun 21 2021, 10:38 PM
rampitec updated this revision to Diff 353677.Jun 22 2021, 9:31 AM
rampitec marked an inline comment as done.

Renamed MaxAlign, fixed tidy formatting warning.

llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
267

Now since we are calling performOptimizedStructLayout(), I am not sure, if the struct member with max align is guarenteed to be at offset 0.

Good catch! No, it is not true anymore. I have removed the comment and renamed the variable. We still need to set FGV alignment to its first field though.

This revision was landed with ongoing or failed builds.Jun 22 2021, 9:58 AM
This revision was automatically updated to reflect the committed changes.
llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
216

This asserts if the type has zero size. There's a repro that hits that internally at 317624. I'm not yet sure what to do with zero size fields.

rampitec added inline comments.Jan 10 2022, 10:46 AM
llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
216

I think we generally do not support opaque types. Anyhow there is no reasonable way to layout this.