[PATCH] D149986: AMDGPU: Force sc0 and sc1 on stores for gfx940 and gfx941

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh added a comment.

I think that if this is a new property of the GFX940/941 targets, and turning it off shouldn't be possible, we shouldn't even bother with a feature and just set a bool in the ST for those targets



================
Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:524
+         SIAtomicAddrSpace::NONE)
+      return enableSC0Bit(MI) | enableSC1Bit(MI);
+    return false;
----------------
kzhuravl wrote:
> jmmartinez wrote:
> > NIT: Is the use of the bitwise or " | " intended? I'd use the logical or " || " instead.
> It is intentional, we need both SC0 and SC1 bits set. If I switch this to || it will short circuit and not invoke enableSC1Bit.
IMHO then it needs a comment to explain that it's intentional, otherwise some innocent maintainer in the future could think it's a typo and change it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149986