[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