[PATCH] D98085: [AMDGPU] Always expand system scope fp atomics on gfx90a

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D98085#2609265 <https://reviews.llvm.org/D98085#2609265>, @rampitec wrote:

> I am looking where to move syncscope initialization and cannot find a place. I do not think I can just initialize it on a first query and cache it because it depends on the LLVMContext. I have also double checked that MF is not created yet and MMI is not created yet too.

Note what would happen if I cache this value at the first call:

  SyncScope::ID LLVMContextImpl::getOrInsertSyncScopeID(StringRef SSN) {
    auto NewSSID = SSC.size();
    assert(NewSSID < std::numeric_limits<SyncScope::ID>::max() &&
           "Hit the maximum number of synchronization scopes allowed!");
    return SSC.insert(std::make_pair(SSN, SyncScope::ID(NewSSID))).first->second;
  }

It will lock numeric value depending on the call history of getOrInsertSyncScopeID(). Caching without mapping to a context will prohibit any part of the compiler to ever call getOrInsertSyncScopeID() and then reuse AMDGPU BE. If the same instance of SIISelLowering will be called with a different module with a different Context cache may contain a wrong value. Besides even that depends on the internal implementation of the Context. AMDGPUMachineModuleInfo is fine do it because it has module context, but anything before MF is created not.


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

https://reviews.llvm.org/D98085