[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