[llvm] [AArch64] Clarify atomic load/store size condition (NFCI) (PR #91907)
via llvm-commits
llvm-commits at lists.llvm.org
Sun May 12 19:27:24 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-aarch64
Author: Nikita Popov (nikic)
<details>
<summary>Changes</summary>
This is currently bailing out on MemSizeInBytes larger than 64 bytes. However, the following code can only handle sizes up to 8 bytes. Possibly there was confusion here between MemSizeInBytes and MemSizeInBits.
I *think* that this can't actually result in an out of bounds read of the opcode table because we'll only ever mark loads/stores of up to 8 bytes as legal (16 byte atomics are custom-legalized earlier). As such, I've changed this condition to an assert.
---
Full diff: https://github.com/llvm/llvm-project/pull/91907.diff
1 Files Affected:
- (modified) llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp (+2-2)
``````````diff
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 61f5bc2464ee5..c681ad347fa88 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -2862,8 +2862,8 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
Order != AtomicOrdering::Unordered &&
Order != AtomicOrdering::Monotonic) {
assert(!isa<GZExtLoad>(LdSt));
- if (MemSizeInBytes > 64)
- return false;
+ assert(MemSizeInBytes <= 8 &&
+ "128-bit atomics should already be custom-legalized");
if (isa<GLoad>(LdSt)) {
static constexpr unsigned LDAPROpcodes[] = {
``````````
</details>
https://github.com/llvm/llvm-project/pull/91907
More information about the llvm-commits
mailing list