[PATCH] D96421: [AMDGPU] Better selection of base offset when merging DS reads/writes

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 11 07:47:09 PST 2021


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:802
   // Try to shift base address to decrease offsets.
-  unsigned OffsetDiff = std::abs((int)EltOffset1 - (int)EltOffset0);
-  CI.BaseOff = std::min(CI.Offset, Paired.Offset);
+  unsigned Min = std::min(EltOffset0, EltOffset1);
+  unsigned Max = std::max(EltOffset0, EltOffset1);
----------------
foad wrote:
> arsenm wrote:
> > Should probably use uint32_t throughout here
> I'd prefer not to, since the rest of the file is pretty consistent in using `unsigned` throughout.
LLVM is pretty consistently wrong in using unsigned for 32-bit values when it's not guaranteed by the standard


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96421



More information about the llvm-commits mailing list