[PATCH] D144759: [AMDGPU] Implement idempotent atomic lowering

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked an inline comment as done.
rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:13371-13377
+  // We do not need to insert a fence here, memory legalizer will do.
+  LoadInst *LI = Builder.CreateAlignedLoad(
+      AI->getType(), AI->getPointerOperand(), AI->getAlign());
+  LI->setAtomic(Order, SSID);
+  LI->copyMetadata(*AI);
+  LI->takeName(AI);
+  AI->replaceAllUsesWith(LI);
----------------
rampitec wrote:
> arsenm wrote:
> > I don't understand why this is a target hook. Why can't this unconditionally happen in the generic code?
> Probably not. The only target implementing it is x86 and it issues target specific intrinsics. It also skips 'or' with zero as it claims to have a better lowering.
Also note that I am skipping the fence on the grounds that memory legalizer will fence it. Otherwise with our address spaces and scopes this would be quite non-trivial and target specific code.


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

https://reviews.llvm.org/D144759