[PATCH] D155051: [AMDGPU] Add sanity check that fixes bad shift operation in AMD backend

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:878
+
   unsigned AllowedBitsForMin = llvm::countr_zero(MaxMask);
   if ((1u << AllowedBitsForMin) <= MinMask)
----------------
konradkusiak97 wrote:
> JonChesterfield wrote:
> > Should this be if masks are equal?
> > 
> > Not sure sanity check describes the condition, maybe drop the comment
> Hm, If the masks are equal, the result will be returning `false`. I think that's the correct behaviour that the author of this function had in mind. That's based on the fact that there is `<=` and not `<` sign in `if ((1u << AllowedBitsForMin) <= MinMask)`, so it really checks if the masks overlap - and two equal masks overlap fully.
If equal masks imply they can't be combined, that should probably be true for the special case of equal masks that are zero.

I haven't looked at the call tree to determine what combining masks means in this context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155051