[PATCH] D123699: [AMDGPU] Remove redundand RequiredAlignment assignment. NFCI.

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 14 01:40:50 PDT 2022


foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:1553
 
       // 12 byte accessing via ds_read/write_b96 require 16-byte alignment on
       // gfx8 and older.
----------------
rampitec wrote:
> foad wrote:
> > Not sure why the comment mentions gfx8. Even on later archs the requirement is for 16-byte alignment **if** you set strict alignment mode.
> 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.
> This is really something like a couple days of work to check with a little benefit.

Understood! :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123699/new/

https://reviews.llvm.org/D123699



More information about the llvm-commits mailing list