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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:11954-11955
+        auto SSID = RMW->getSyncScopeID();
+        if (SSID == SyncScope::System ||
+            SSID == RMW->getContext().getOrInsertSyncScopeID("one-as"))
+          return AtomicExpansionKind::CmpXChg;
----------------
arsenm wrote:
> rampitec wrote:
> > arsenm wrote:
> > > rampitec wrote:
> > > > arsenm wrote:
> > > > > Isn't there some scope greater or equal function? Should avoid directly referencing one-as
> > > > It is, but it is located in the MMI whcih is not yet available here. In fact I don't even have MF yet to query it.
> > > > 
> > > > Anyway since it is specific to the only target and that target does not have a higher scope I suppose it shall be OK.
> > > Can it be moved out of the MMI? That sounds like a weird place for it
> > Any good ideas where to? It needs an LLVMContext, so definitely something which has access to Module.
> It seems like its a target specific context, its own thing separate from MMI. I guess we don't have anything like that now, but I would think it would look like a function returning a struct initialized on the first call
Technically it depends on the target (scope ordering) and not on a subtarget. At least so far and at least so far we do not envision so drastic changes that would require a subtarget differentiation of the scopes. I guess we would resist as much as we can if that is proposed. So technically it should not need MF, only the Module.


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

https://reviews.llvm.org/D98085