[PATCH] D84522: [AMDGPU] Merge UnalignedBufferAccess into UnalignedAccessMode

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:90
 
-def FeatureUnalignedBufferAccess : SubtargetFeature<"unaligned-buffer-access",
-  "UnalignedBufferAccess",
----------------
I think we still need something to indicate the hardware supports the unaligned global/buffer memory vs. the driver set the config register to use it.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:93
-  "true",
-  "Support unaligned global loads and stores"
->;
----------------
Hardware support for unaligned global loads and stores


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:107
   "true",
-  "Support unaligned local and region loads and stores"
+  "Support unaligned global, local and region loads and stores"
 >;
----------------
Maybe change "Support" to "Enable" since this is the "dynamic" control. Also could make it clearer that this doesn't allow these on targets that don't support that specific memory space.


Should also probably move this down to the section under // Subtarget Features (options and debugging)



================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:1449
 
-  if (Subtarget->hasUnalignedBufferAccess() &&
+  if (Subtarget->hasUnalignedAccessMode() &&
       !(AddrSpace == AMDGPUAS::LOCAL_ADDRESS ||
----------------
Probably needs to logically be hasUnalignedAccessMode && hardwareSupportsUnalignedGlobal (unless we somehow force off the dynamic mode feature if the hardware doesn't support it). maybe this should follow the model of ECC/XNACK


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

https://reviews.llvm.org/D84522