[PATCH] D139551: [AMDGPU] Use CAS loop for min/max atomics at system scope

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 7 09:27:54 PST 2022


arsenm requested changes to this revision.
arsenm added a comment.
This revision now requires changes to proceed.

Most of these test changes break the purpose of the tests, which was proper legalization handling of the atomic instructions



================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12896-12897
 
+  bool HasGlobalOrFlatAS =
+      AS == AMDGPUAS::GLOBAL_ADDRESS || AS == AMDGPUAS::FLAT_ADDRESS;
+
----------------
Should use AMDGPU::isFlatGlobalAddrSpace, in principal the same should apply to any other random address space number


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12903-12906
+  bool UnsafeFPAtomicsDisabled =
+      RMW->getFunction()
+          ->getFnAttribute("amdgpu-unsafe-fp-atomics")
+          .getValueAsString() != "true";
----------------
move to predicate function to call at the use sites?


================
Comment at: llvm/test/CodeGen/AMDGPU/dag-divergence-atomic.ll:184
   %n64 = zext i32 %n32 to i64
   %p1 = getelementptr inbounds %S, %S addrspace(1)* %q, i64 %n64, i32 0
   store float 1.0, float addrspace(1)* %p1
----------------
This breaks the purpose of this test. These should be updated to have a scope that doesn't require the cas loop


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139551



More information about the llvm-commits mailing list