[llvm] [AArch64] Clarify atomic load/store size condition (NFCI) (PR #91907)

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Sun May 12 19:26:56 PDT 2024


https://github.com/nikic created https://github.com/llvm/llvm-project/pull/91907

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.

>From 7f166d12659e01040da1a2c08176b900877bdc31 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Mon, 13 May 2024 11:15:52 +0900
Subject: [PATCH] [AArch64] Clarify atomic load/store size condition (NFCI)

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 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.
---
 llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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[] = {



More information about the llvm-commits mailing list