Use CAS loop for min/max atomics at system scope. Currently a hardware atomic instruction is being used which does not work at system scope.
Details
Diff Detail
Event Timeline
Most of these test changes break the purpose of the tests, which was proper legalization handling of the atomic instructions
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | ||
---|---|---|
12896–12897 | Should use AMDGPU::isFlatGlobalAddrSpace, in principal the same should apply to any other random address space number | |
12903–12906 | move to predicate function to call at the use sites? | |
llvm/test/CodeGen/AMDGPU/dag-divergence-atomic.ll | ||
206 | This breaks the purpose of this test. These should be updated to have a scope that doesn't require the cas loop |
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | ||
---|---|---|
12896–12897 | This check here is just a factoring out of the switch statement since now this check is done in 2 places. Are you implying that this check was wrong or insufficient before? | |
12903–12906 | Could you please give me more details regarding this request? Again this is a check which was factored out of the switch statement because the same check is now done in two different locations. | |
llvm/test/CodeGen/AMDGPU/dag-divergence-atomic.ll | ||
206 | I agree, this is a good point and I'll try to fix it. |
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | ||
---|---|---|
12896–12897 | Yes | |
12903–12906 | bool unsafeAtomicsDisabled(const Function &F) { return .... } and call that at the use sites rather than eagerly parsing the attribute |
I think you're best off replacing all changed tests with a specific scope, and introducing new tests specifically for system scoped atomics
@arsenm I have updated the tests and made one of the two code changes you suggested. I have to do the function still.
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | ||
---|---|---|
12986 | I'm pretty sure all of the atomics (except maybe add?) have the same issue, but they don't all need to change at once |
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | ||
---|---|---|
12986 | I have the same feeling. |
static and const&