Set MaxAtomicSizeInBitsSupported for remaining targets. #75703
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Targets affected: - NVPTX and BPF: set to 64 bits. - ARC, Lanai, and MSP430: set to 0 (they don't implement atomics). Those which didn't yet add AtomicExpandPass to their pass pipeline now do so. This will result in larger atomic operations getting expanded to `__atomic_*` libcalls via AtomicExpandPass. On all these targets, this now matches what Clang already does in the frontend. The only targets which do not configure AtomicExpandPass now are: - DirectX and SPIRV: they aren't normal backends. - AVR: a single-cpu architecture with no privileged/user divide, which implements all atomics by disabling/enabling interrupts. Thus, typical constraints on size and alignment aren't necessary.
|
@llvm/pr-subscribers-backend-msp430 Author: James Y Knight (jyknight) ChangesTargets affected:
Those which didn't yet add AtomicExpandPass to their pass pipeline now do so. This will result in larger atomic operations getting expanded to The only targets which do not configure AtomicExpandPass now are:
Full diff: https://github.com/llvm/llvm-project/pull/75703.diff 14 Files Affected:
diff --git a/llvm/lib/Target/ARC/ARCISelLowering.cpp b/llvm/lib/Target/ARC/ARCISelLowering.cpp
index 5d9a366f5ed540..fb1a42440498a1 100644
--- a/llvm/lib/Target/ARC/ARCISelLowering.cpp
+++ b/llvm/lib/Target/ARC/ARCISelLowering.cpp
@@ -174,6 +174,8 @@ ARCTargetLowering::ARCTargetLowering(const TargetMachine &TM,
setOperationAction(ISD::READCYCLECOUNTER, MVT::i32, Legal);
setOperationAction(ISD::READCYCLECOUNTER, MVT::i64,
isTypeLegal(MVT::i64) ? Legal : Custom);
+
+ setMaxAtomicSizeInBitsSupported(0);
}
const char *ARCTargetLowering::getTargetNodeName(unsigned Opcode) const {
diff --git a/llvm/lib/Target/ARC/ARCTargetMachine.cpp b/llvm/lib/Target/ARC/ARCTargetMachine.cpp
index d4ae3255b32abf..4f612ae623b986 100644
--- a/llvm/lib/Target/ARC/ARCTargetMachine.cpp
+++ b/llvm/lib/Target/ARC/ARCTargetMachine.cpp
@@ -57,6 +57,7 @@ class ARCPassConfig : public TargetPassConfig {
return getTM<ARCTargetMachine>();
}
+ void addIRPasses() override;
bool addInstSelector() override;
void addPreEmitPass() override;
void addPreRegAlloc() override;
@@ -68,6 +69,12 @@ TargetPassConfig *ARCTargetMachine::createPassConfig(PassManagerBase &PM) {
return new ARCPassConfig(*this, PM);
}
+void ARCPassConfig::addIRPasses() {
+ addPass(createAtomicExpandPass());
+
+ TargetPassConfig::addIRPasses();
+}
+
bool ARCPassConfig::addInstSelector() {
addPass(createARCISelDag(getARCTargetMachine(), getOptLevel()));
return false;
diff --git a/llvm/lib/Target/BPF/BPFISelLowering.cpp b/llvm/lib/Target/BPF/BPFISelLowering.cpp
index 2fe86e75ddae8f..4d8ace7c1ece02 100644
--- a/llvm/lib/Target/BPF/BPFISelLowering.cpp
+++ b/llvm/lib/Target/BPF/BPFISelLowering.cpp
@@ -151,6 +151,7 @@ BPFTargetLowering::BPFTargetLowering(const TargetMachine &TM,
}
setBooleanContents(ZeroOrOneBooleanContent);
+ setMaxAtomicSizeInBitsSupported(64);
// Function alignments
setMinFunctionAlignment(Align(8));
diff --git a/llvm/lib/Target/BPF/BPFTargetMachine.cpp b/llvm/lib/Target/BPF/BPFTargetMachine.cpp
index ab0db576f7f72d..7c83225c5d9345 100644
--- a/llvm/lib/Target/BPF/BPFTargetMachine.cpp
+++ b/llvm/lib/Target/BPF/BPFTargetMachine.cpp
@@ -148,7 +148,9 @@ void BPFTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
}
void BPFPassConfig::addIRPasses() {
+ addPass(createAtomicExpandPass());
addPass(createBPFCheckAndAdjustIR());
+
TargetPassConfig::addIRPasses();
}
diff --git a/llvm/lib/Target/Lanai/LanaiISelLowering.cpp b/llvm/lib/Target/Lanai/LanaiISelLowering.cpp
index cbb5c2b998e27a..7e6235942619cf 100644
--- a/llvm/lib/Target/Lanai/LanaiISelLowering.cpp
+++ b/llvm/lib/Target/Lanai/LanaiISelLowering.cpp
@@ -166,6 +166,8 @@ LanaiTargetLowering::LanaiTargetLowering(const TargetMachine &TM,
// Booleans always contain 0 or 1.
setBooleanContents(ZeroOrOneBooleanContent);
+
+ setMaxAtomicSizeInBitsSupported(0);
}
SDValue LanaiTargetLowering::LowerOperation(SDValue Op,
diff --git a/llvm/lib/Target/Lanai/LanaiTargetMachine.cpp b/llvm/lib/Target/Lanai/LanaiTargetMachine.cpp
index 039182b3ffe60b..33479720183b43 100644
--- a/llvm/lib/Target/Lanai/LanaiTargetMachine.cpp
+++ b/llvm/lib/Target/Lanai/LanaiTargetMachine.cpp
@@ -93,6 +93,7 @@ class LanaiPassConfig : public TargetPassConfig {
return getTM<LanaiTargetMachine>();
}
+ void addIRPasses() override;
bool addInstSelector() override;
void addPreSched2() override;
void addPreEmitPass() override;
@@ -104,6 +105,12 @@ LanaiTargetMachine::createPassConfig(PassManagerBase &PassManager) {
return new LanaiPassConfig(*this, &PassManager);
}
+void LanaiPassConfig::addIRPasses() {
+ addPass(createAtomicExpandPass());
+
+ TargetPassConfig::addIRPasses();
+}
+
// Install an instruction selector pass.
bool LanaiPassConfig::addInstSelector() {
addPass(createLanaiISelDag(getLanaiTargetMachine()));
diff --git a/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp b/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
index ee7762c296bf50..9422bcca2299ed 100644
--- a/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
+++ b/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
@@ -333,6 +333,7 @@ MSP430TargetLowering::MSP430TargetLowering(const TargetMachine &TM,
setMinFunctionAlignment(Align(2));
setPrefFunctionAlignment(Align(2));
+ setMaxAtomicSizeInBitsSupported(0);
}
SDValue MSP430TargetLowering::LowerOperation(SDValue Op,
diff --git a/llvm/lib/Target/MSP430/MSP430TargetMachine.cpp b/llvm/lib/Target/MSP430/MSP430TargetMachine.cpp
index 39e0658eb70dd5..283de46e57d5c4 100644
--- a/llvm/lib/Target/MSP430/MSP430TargetMachine.cpp
+++ b/llvm/lib/Target/MSP430/MSP430TargetMachine.cpp
@@ -65,6 +65,7 @@ class MSP430PassConfig : public TargetPassConfig {
return getTM<MSP430TargetMachine>();
}
+ void addIRPasses() override;
bool addInstSelector() override;
void addPreEmitPass() override;
};
@@ -81,6 +82,12 @@ MachineFunctionInfo *MSP430TargetMachine::createMachineFunctionInfo(
F, STI);
}
+void MSP430PassConfig::addIRPasses() {
+ addPass(createAtomicExpandPass());
+
+ TargetPassConfig::addIRPasses();
+}
+
bool MSP430PassConfig::addInstSelector() {
// Install an instruction selector.
addPass(createMSP430ISelDag(getMSP430TargetMachine(), getOptLevel()));
diff --git a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
index e8f36bf50a1b08..de6de3214521e2 100644
--- a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
@@ -854,6 +854,7 @@ NVPTXTargetLowering::NVPTXTargetLowering(const NVPTXTargetMachine &TM,
computeRegisterProperties(STI.getRegisterInfo());
setMinCmpXchgSizeInBits(32);
+ setMaxAtomicSizeInBitsSupported(64);
}
const char *NVPTXTargetLowering::getTargetNodeName(unsigned Opcode) const {
diff --git a/llvm/test/CodeGen/ARC/atomic-oversize.ll b/llvm/test/CodeGen/ARC/atomic-oversize.ll
new file mode 100644
index 00000000000000..678c1ae649c6a8
--- /dev/null
+++ b/llvm/test/CodeGen/ARC/atomic-oversize.ll
@@ -0,0 +1,11 @@
+; RUN: llc -mtriple=arc < %s | FileCheck %s
+
+; Native atomics are unsupported, so all are oversize.
+define void @test(ptr %a) nounwind {
+; CHECK-LABEL: test:
+; CHECK: bl @__atomic_load_1
+; CHECK: bl @__atomic_store_1
+ %1 = load atomic i8, ptr %a seq_cst, align 16
+ store atomic i8 %1, ptr %a seq_cst, align 16
+ ret void
+}
diff --git a/llvm/test/CodeGen/BPF/atomic-oversize.ll b/llvm/test/CodeGen/BPF/atomic-oversize.ll
new file mode 100644
index 00000000000000..187f0964d4fb83
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/atomic-oversize.ll
@@ -0,0 +1,12 @@
+; RUN: llc -mtriple=bpf < %s | FileCheck %s
+; XFAIL: *
+; Doesn't currently build, with error 'only small returns supported'.
+
+define void @test(ptr %a) nounwind {
+; CHECK-LABEL: test:
+; CHECK: call __atomic_load_16
+; CHECK: call __atomic_store_16
+ %1 = load atomic i128, ptr %a monotonic, align 16
+ store atomic i128 %1, ptr %a monotonic, align 16
+ ret void
+}
diff --git a/llvm/test/CodeGen/Lanai/atomic-oversize.ll b/llvm/test/CodeGen/Lanai/atomic-oversize.ll
new file mode 100644
index 00000000000000..93307ac8184d08
--- /dev/null
+++ b/llvm/test/CodeGen/Lanai/atomic-oversize.ll
@@ -0,0 +1,11 @@
+; RUN: llc -mtriple=lanai < %s | FileCheck %s
+
+; Native atomics are unsupported, so all are oversize.
+define void @test(ptr %a) nounwind {
+; CHECK-LABEL: test:
+; CHECK: bt __atomic_load_1
+; CHECK: bt __atomic_store_1
+ %1 = load atomic i8, ptr %a monotonic, align 16
+ store atomic i8 %1, ptr %a monotonic, align 16
+ ret void
+}
diff --git a/llvm/test/CodeGen/MSP430/atomic-oversize.ll b/llvm/test/CodeGen/MSP430/atomic-oversize.ll
new file mode 100644
index 00000000000000..53b668ab25b569
--- /dev/null
+++ b/llvm/test/CodeGen/MSP430/atomic-oversize.ll
@@ -0,0 +1,11 @@
+; RUN: llc -mtriple=msp430 < %s | FileCheck %s
+
+; Native atomics are unsupported, so all are oversize.
+define void @test(ptr %a) nounwind {
+; CHECK-LABEL: test:
+; CHECK: call #__atomic_load_1
+; CHECK: call #__atomic_store_1
+ %1 = load atomic i8, ptr %a monotonic, align 16
+ store atomic i8 %1, ptr %a monotonic, align 16
+ ret void
+}
diff --git a/llvm/test/CodeGen/NVPTX/atomicrmw-expand.ll b/llvm/test/CodeGen/NVPTX/atomicrmw-expand.ll
index d4fd620592048c..b65c281092dd72 100644
--- a/llvm/test/CodeGen/NVPTX/atomicrmw-expand.ll
+++ b/llvm/test/CodeGen/NVPTX/atomicrmw-expand.ll
@@ -140,26 +140,30 @@ entry:
ret void
}
-; TODO: We might still want to test other types, such as i128. Currently the
-; backend doesn't support them. Atomic expand only supports expansion to cas of
-; the same bitwidth, which means even after expansion, the back end still
-; doesn't support the instruction. Here we still put the tests. Remove the
-; comment once we have proper support, either from atomic expand or backend.
-
-; define void @bitwise_i128(ptr %0, i128 %1) {
-; entry:
-; %2 = atomicrmw and ptr %0, i128 %1 monotonic, align 16
-; %3 = atomicrmw or ptr %0, i128 %1 monotonic, align 16
-; %4 = atomicrmw xor ptr %0, i128 %1 monotonic, align 16
-; %5 = atomicrmw xchg ptr %0, i128 %1 monotonic, align 16
-; ret void
-; }
+; CHECK-LABEL: bitwise_i128
+define void @bitwise_i128(ptr %0, i128 %1) {
+entry:
+ ; ALL: __atomic_fetch_and_16
+ %2 = atomicrmw and ptr %0, i128 %1 monotonic, align 16
+ ; ALL: __atomic_fetch_or_16
+ %3 = atomicrmw or ptr %0, i128 %1 monotonic, align 16
+ ; ALL: __atomic_fetch_xor_16
+ %4 = atomicrmw xor ptr %0, i128 %1 monotonic, align 16
+ ; ALL: __atomic_exchange_16
+ %5 = atomicrmw xchg ptr %0, i128 %1 monotonic, align 16
+ ret void
+}
-; define void @minmax_i128(ptr %0, i128 %1) {
-; entry:
-; %2 = atomicrmw min ptr %0, i128 %1 monotonic, align 16
-; %3 = atomicrmw max ptr %0, i128 %1 monotonic, align 16
-; %4 = atomicrmw umin ptr %0, i128 %1 monotonic, align 16
-; %5 = atomicrmw umax ptr %0, i128 %1 monotonic, align 16
-; ret void
-; }
+; CHECK-LABEL: minmax_i128
+define void @minmax_i128(ptr %0, i128 %1) {
+entry:
+ ; ALL: __atomic_compare_exchange_16
+ %2 = atomicrmw min ptr %0, i128 %1 monotonic, align 16
+ ; ALL: __atomic_compare_exchange_16
+ %3 = atomicrmw max ptr %0, i128 %1 monotonic, align 16
+ ; ALL: __atomic_compare_exchange_16
+ %4 = atomicrmw umin ptr %0, i128 %1 monotonic, align 16
+ ; ALL: __atomic_compare_exchange_16
+ %5 = atomicrmw umax ptr %0, i128 %1 monotonic, align 16
+ ret void
+}
|
|
Ping! |
|
@jyknight I believe MSP430 should be pretty similar with AVR here. But let me think a bit about how the atomics should be structured there (I have a long-standing TODO in my list for this) |
|
Since this change makes LLVM consistent with what already happens in the Clang frontend for MSP430, I think it makes most sense to submit this, and do such an effort of reconsidering how it should work as a future task. I'm happy to file a bug and assign it to you if you like (or reference such a bug in the commit description if you file it). |
|
@jyknight Yes, sure. In this sense the change is ok for me. |
jpienaar
approved these changes
Jan 4, 2024
|
In further investigation I realized AVR is actually only partially functional today; the backend is broken if you attempt to use larger sized atomic operations. I now expect to also convert it to use AtomicExpandPass, but with NotAtomic lowering, and custom "fences" for disabling/enabling interrupts. That'll be future work; I'll continue to leave it untouched here. |
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
Targets affected: - NVPTX and BPF: set to 64 bits. - ARC, Lanai, and MSP430: set to 0 (they don't implement atomics). Those which didn't yet add AtomicExpandPass to their pass pipeline now do so. This will result in larger atomic operations getting expanded to `__atomic_*` libcalls via AtomicExpandPass. On all these targets, this now matches what Clang already does in the frontend. The only targets which do not configure AtomicExpandPass now are: - DirectX and SPIRV: they aren't normal backends. - AVR: a single-cpu architecture with no privileged/user divide, which could implement all atomics by disabling/enabling interrupts, regardless of size/alignment. Will be addressed by future work.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Targets affected:
Those which didn't yet add AtomicExpandPass to their pass pipeline now do so.
This will result in larger atomic operations getting expanded to
__atomic_*libcalls via AtomicExpandPass. On all these targets, this now matches what Clang already does in the frontend.The only targets which do not configure AtomicExpandPass now are: