[PATCH] D84403: [AMDGPU] Use ds_read/write_b96/b128 when possible for SDag

Thu Aug 20 04:46:04 PDT 2020

mbrkusanin added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:8412
+    if (Subtarget->hasDS96AndDS128() &&
+        ((Subtarget->useDS128() && VT.getStoreSize() == 16) ||
+         (VT.getStoreSize() == 12)) &&
----------------
arsenm wrote:
> Whether usesDS128 should apply to B96 is open for interpretation (although I'm not sure we even need this toggleable feature anymore)
It looks like it was introduced because of https://bugs.freedesktop.org/show_bug.cgi?id=105464 to avoid a bug with some 128bit vectors but it was fixed in the meantime and this option is now enabled by default. Not sure if the same bug affected b96 or not. Either way if there is no need for this feature anymore it might make more sense to remove it then expand on it. 
Reason for hasDS96AndDS128() was to avoid expanding on that one.

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

https://reviews.llvm.org/D84403