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

arsenm added inline comments. Comment at: llvm/lib/TargetParser/TargetParser.cpp:330-332 case GK_GFX940: + Features["force-store-sc0-sc1"] = true; + [[fallthrough]]; I don't see a reason to set this here. There's no need to expose this to the IR.

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

scott.linder added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:517-529 + bool tryForceStoreSC0SC1(const SIMemOpInfo , + MachineBasicBlock::iterator ) const override { +if (ST.hasForceStoreSC0SC1() && +

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

kzhuravl added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:524 + SIAtomicAddrSpace::NONE) + return enableSC0Bit(MI) | enableSC1Bit(MI); +return false; Pierre-vh wrote: > kzhuravl wrote: > > jmmartinez wrote: > > >

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

kzhuravl marked an inline comment as done. kzhuravl added a comment. In D149986#4334274 , @Pierre-vh wrote: > 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

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

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:

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

kzhuravl marked an inline comment as done. kzhuravl added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:524 + SIAtomicAddrSpace::NONE) + return enableSC0Bit(MI) | enableSC1Bit(MI); +return false; jmmartinez wrote: >

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

jmmartinez added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:524 + SIAtomicAddrSpace::NONE) + return enableSC0Bit(MI) | enableSC1Bit(MI); +return false; NIT: Is the use of the bitwise or " | " intended? I'd use the

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

kzhuravl added a comment. In D149986#4329302 , @arsenm wrote: > Should this be a feature set by default in the subtarget constructor instead? > Should you be able to turn this off? Users should not be able to turn this off. If user wants it off, user

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

arsenm added a comment. Should this be a feature set by default in the subtarget constructor instead? Should you be able to turn this off? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149986/new/ https://reviews.llvm.org/D149986