This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Remove redundand RequiredAlignment assignment. NFCI.
ClosedPublic

Authored by rampitec on Apr 13 2022, 11:20 AM.

Diff Detail

Event Timeline

rampitec created this revision.Apr 13 2022, 11:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 11:20 AM
rampitec requested review of this revision.Apr 13 2022, 11:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 11:20 AM
Herald added a subscriber: wdng. · View Herald Transcript
foad accepted this revision.Apr 14 2022, 1:24 AM

Thanks!

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
1553

Not sure why the comment mentions gfx8. Even on later archs the requirement is for 16-byte alignment if you set strict alignment mode.

This revision is now accepted and ready to land.Apr 14 2022, 1:24 AM
rampitec added inline comments.Apr 14 2022, 1:37 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
1553

I left it as is, but it suggests on VI this was required even with unaligned access enabled. From my recollections this is not true, but I would not bet on it until I dig the specs. Moreover, I believe unaligned access was legal with a proper setting since CI, but that needs a doc excavations. I would have to dig around the shelve to find a Bonaire or Fiji to check, so I just left it as is for now. This is really something like a couple days of work to check with a little benefit.

foad added inline comments.Apr 14 2022, 1:40 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
1553

This is really something like a couple days of work to check with a little benefit.

Understood! :)

rampitec added inline comments.Apr 14 2022, 1:58 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
1553

Moreover, I shall have Fiji around, Bonaire in a radius of 60 miles and I don't even remember if I have a Tahiti handy anywhere. Too much for fixing someone's comment ;)

This revision was landed with ongoing or failed builds.Apr 14 2022, 2:04 AM
This revision was automatically updated to reflect the committed changes.