[llvm] [ARM][MC] Add GNU Alias for ldrexd and strexd instructions (PR #86507)
Alfie Richards via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 26 06:47:34 PDT 2024
https://github.com/AlfieRichardsArm updated https://github.com/llvm/llvm-project/pull/86507
>From ccb33b2a2a236a5c90f405c33f84c466fee3f59f Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Mon, 25 Mar 2024 14:16:58 +0000
Subject: [PATCH 1/3] [ARM][MC] Add GNU Alias for ldrexd and strexd
instructions.
These aliasses were supported previously in llvm-mc but support was
lost at some point and for a while it seems to have caused a crash.
This adds back the alternate forms and tidies up this section of
code a little.
---
.../lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 40 ++++++++++++-------
llvm/test/MC/ARM/basic-arm-instructions.s | 8 ++++
2 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 2ad576ab1a9caa..b50698996e62a5 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -7343,34 +7343,46 @@ bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
// automatically
// expressed as a GPRPair, so we have to manually merge them.
// FIXME: We would really like to be able to tablegen'erate this.
- if (!isThumb() && Operands.size() > MnemonicOpsEndInd + 1 &&
+ bool IsLoad = (Mnemonic == "ldrexd" || Mnemonic == "ldaexd");
+ if (!isThumb() && Operands.size() > MnemonicOpsEndInd + 1 + (!IsLoad) &&
(Mnemonic == "ldrexd" || Mnemonic == "strexd" || Mnemonic == "ldaexd" ||
Mnemonic == "stlexd")) {
- bool isLoad = (Mnemonic == "ldrexd" || Mnemonic == "ldaexd");
- unsigned Idx = isLoad ? MnemonicOpsEndInd : MnemonicOpsEndInd + 1;
+ unsigned Idx = IsLoad ? MnemonicOpsEndInd : MnemonicOpsEndInd + 1;
ARMOperand &Op1 = static_cast<ARMOperand &>(*Operands[Idx]);
ARMOperand &Op2 = static_cast<ARMOperand &>(*Operands[Idx + 1]);
const MCRegisterClass &MRC = MRI->getRegClass(ARM::GPRRegClassID);
- // Adjust only if Op1 and Op2 are GPRs.
- if (Op1.isReg() && Op2.isReg() && MRC.contains(Op1.getReg()) &&
- MRC.contains(Op2.getReg())) {
+ bool IsGNUAlias = !(Op2.isReg() && MRC.contains(Op2.getReg()));
+ // Adjust only if Op1 is a GPR.
+ if (Op1.isReg() && MRC.contains(Op1.getReg())) {
unsigned Reg1 = Op1.getReg();
- unsigned Reg2 = Op2.getReg();
unsigned Rt = MRI->getEncodingValue(Reg1);
- unsigned Rt2 = MRI->getEncodingValue(Reg2);
- // Rt2 must be Rt + 1 and Rt must be even.
- if (Rt + 1 != Rt2 || (Rt & 1)) {
- return Error(Op2.getStartLoc(),
- isLoad ? "destination operands must be sequential"
- : "source operands must be sequential");
+ // Check we are not in the GNU alias case with only one of the register
+ // pair specified
+ if (!IsGNUAlias) {
+ unsigned Reg2 = Op2.getReg();
+ unsigned Rt2 = MRI->getEncodingValue(Reg2);
+ // Rt2 must be Rt + 1.
+ if (Rt + 1 != Rt2)
+ return Error(Op2.getStartLoc(),
+ IsLoad ? "destination operands must be sequential"
+ : "source operands must be sequential");
}
+ // Rt bust be even
+ if (Rt & 1)
+ return Error(
+ Op1.getStartLoc(),
+ IsLoad ? "destination operands must start start at an even register"
+ : "source operands must start start at an even register");
+
unsigned NewReg = MRI->getMatchingSuperReg(
Reg1, ARM::gsub_0, &(MRI->getRegClass(ARM::GPRPairRegClassID)));
Operands[Idx] =
ARMOperand::CreateReg(NewReg, Op1.getStartLoc(), Op2.getEndLoc());
- Operands.erase(Operands.begin() + Idx + 1);
+ // Only remove redundent operand if not in GNU alias case
+ if (!IsGNUAlias)
+ Operands.erase(Operands.begin() + Idx + 1);
}
}
diff --git a/llvm/test/MC/ARM/basic-arm-instructions.s b/llvm/test/MC/ARM/basic-arm-instructions.s
index 84a7cf52fa30e0..9f3a5cd4afa792 100644
--- a/llvm/test/MC/ARM/basic-arm-instructions.s
+++ b/llvm/test/MC/ARM/basic-arm-instructions.s
@@ -1202,6 +1202,10 @@ Lforward:
@ CHECK: ldrex r1, [r7] @ encoding: [0x9f,0x1f,0x97,0xe1]
@ CHECK: ldrexd r6, r7, [r8] @ encoding: [0x9f,0x6f,0xb8,0xe1]
+@ GNU alias
+ ldrexd r6, [r8]
+@ CHECK: ldrexd r6, r7, [r8] @ encoding: [0x9f,0x6f,0xb8,0xe1]
+
@------------------------------------------------------------------------------
@ LDRHT
@------------------------------------------------------------------------------
@@ -2904,6 +2908,10 @@ Lforward:
@ CHECK: strex r2, r1, [r7] @ encoding: [0x91,0x2f,0x87,0xe1]
@ CHECK: strexd r6, r2, r3, [r8] @ encoding: [0x92,0x6f,0xa8,0xe1]
+@ GNU alias
+ strexd r6, r2, [r8]
+@ CHECK: strexd r6, r2, r3, [r8] @ encoding: [0x92,0x6f,0xa8,0xe1]
+
@------------------------------------------------------------------------------
@ STR
@------------------------------------------------------------------------------
>From b886a86c3971c57b78054024b2e8e21bde6e6b25 Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Mon, 25 Mar 2024 14:29:02 +0000
Subject: [PATCH 2/3] Spelling
---
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index b50698996e62a5..2dab6137ed27ce 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -7369,7 +7369,7 @@ bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
IsLoad ? "destination operands must be sequential"
: "source operands must be sequential");
}
- // Rt bust be even
+ // Rt must be even
if (Rt & 1)
return Error(
Op1.getStartLoc(),
@@ -7380,7 +7380,7 @@ bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
Reg1, ARM::gsub_0, &(MRI->getRegClass(ARM::GPRPairRegClassID)));
Operands[Idx] =
ARMOperand::CreateReg(NewReg, Op1.getStartLoc(), Op2.getEndLoc());
- // Only remove redundent operand if not in GNU alias case
+ // redundant operand only exists if not in GNU alias case
if (!IsGNUAlias)
Operands.erase(Operands.begin() + Idx + 1);
}
>From c28af19a2a5d2022f295f08e97827bd4244a4f7d Mon Sep 17 00:00:00 2001
From: Alfie Richards <alfie.richards at arm.com>
Date: Tue, 26 Mar 2024 13:47:01 +0000
Subject: [PATCH 3/3] Cleanup the handling on GNU aliasses and add some more
tests
---
.../lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 49 +++++++++----------
.../ARM/load-store-acquire-release-v8-thumb.s | 8 +++
.../MC/ARM/load-store-acquire-release-v8.s | 17 ++++++-
3 files changed, 48 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 2dab6137ed27ce..b2ceabfb354ade 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -6865,7 +6865,7 @@ static void applyMnemonicAliases(StringRef &Mnemonic,
const FeatureBitset &Features,
unsigned VariantID);
-// The GNU assembler has aliases of ldrd and strd with the second register
+// The GNU assembler has aliases of ldrd, strd, ldrexd, strexd, ldaexd, and stlexd with the second register
// omitted. We don't have a way to do that in tablegen, so fix it up here.
//
// We have to be careful to not emit an invalid Rt2 here, because the rest of
@@ -6876,13 +6876,18 @@ static void applyMnemonicAliases(StringRef &Mnemonic,
void ARMAsmParser::fixupGNULDRDAlias(StringRef Mnemonic,
OperandVector &Operands,
unsigned MnemonicOpsEndInd) {
- if (Mnemonic != "ldrd" && Mnemonic != "strd")
- return;
- if (Operands.size() < MnemonicOpsEndInd + 2)
+ if (Mnemonic != "ldrd" && Mnemonic != "strd" &&
+ Mnemonic != "ldrexd" && Mnemonic != "strexd" &&
+ Mnemonic != "ldaexd" && Mnemonic != "stlexd")
return;
- ARMOperand &Op2 = static_cast<ARMOperand &>(*Operands[MnemonicOpsEndInd]);
- ARMOperand &Op3 = static_cast<ARMOperand &>(*Operands[MnemonicOpsEndInd + 1]);
+ unsigned IdX = Mnemonic == "strexd" || Mnemonic == "stlexd" ? MnemonicOpsEndInd + 1 : MnemonicOpsEndInd;
+
+ if (Operands.size() < IdX + 2)
+ return;
+
+ ARMOperand &Op2 = static_cast<ARMOperand &>(*Operands[IdX]);
+ ARMOperand &Op3 = static_cast<ARMOperand &>(*Operands[IdX + 1]);
if (!Op2.isReg())
return;
@@ -6907,7 +6912,7 @@ void ARMAsmParser::fixupGNULDRDAlias(StringRef Mnemonic,
return;
Operands.insert(
- Operands.begin() + MnemonicOpsEndInd + 1,
+ Operands.begin() + IdX + 1,
ARMOperand::CreateReg(PairedReg, Op2.getStartLoc(), Op2.getEndLoc()));
}
@@ -7336,6 +7341,9 @@ bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
static_cast<ARMOperand &>(*Operands[MnemonicOpsEndInd]).isImm())
removeCondCode(Operands, MnemonicOpsEndInd);
+ // GNU Assembler extension (compatibility).
+ fixupGNULDRDAlias(Mnemonic, Operands, MnemonicOpsEndInd);
+
// Adjust operands of ldrexd/strexd to MCK_GPRPair.
// ldrexd/strexd require even/odd GPR pair. To enforce this constraint,
// a single GPRPair reg operand is used in the .td file to replace the two
@@ -7352,23 +7360,18 @@ bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
ARMOperand &Op2 = static_cast<ARMOperand &>(*Operands[Idx + 1]);
const MCRegisterClass &MRC = MRI->getRegClass(ARM::GPRRegClassID);
- bool IsGNUAlias = !(Op2.isReg() && MRC.contains(Op2.getReg()));
// Adjust only if Op1 is a GPR.
if (Op1.isReg() && MRC.contains(Op1.getReg())) {
unsigned Reg1 = Op1.getReg();
unsigned Rt = MRI->getEncodingValue(Reg1);
+ unsigned Reg2 = Op2.getReg();
+ unsigned Rt2 = MRI->getEncodingValue(Reg2);
+ // Rt2 must be Rt + 1.
+ if (Rt + 1 != Rt2)
+ return Error(Op2.getStartLoc(),
+ IsLoad ? "destination operands must be sequential"
+ : "source operands must be sequential");
- // Check we are not in the GNU alias case with only one of the register
- // pair specified
- if (!IsGNUAlias) {
- unsigned Reg2 = Op2.getReg();
- unsigned Rt2 = MRI->getEncodingValue(Reg2);
- // Rt2 must be Rt + 1.
- if (Rt + 1 != Rt2)
- return Error(Op2.getStartLoc(),
- IsLoad ? "destination operands must be sequential"
- : "source operands must be sequential");
- }
// Rt must be even
if (Rt & 1)
return Error(
@@ -7378,17 +7381,13 @@ bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
unsigned NewReg = MRI->getMatchingSuperReg(
Reg1, ARM::gsub_0, &(MRI->getRegClass(ARM::GPRPairRegClassID)));
+
Operands[Idx] =
ARMOperand::CreateReg(NewReg, Op1.getStartLoc(), Op2.getEndLoc());
- // redundant operand only exists if not in GNU alias case
- if (!IsGNUAlias)
- Operands.erase(Operands.begin() + Idx + 1);
+ Operands.erase(Operands.begin() + Idx + 1);
}
}
- // GNU Assembler extension (compatibility).
- fixupGNULDRDAlias(Mnemonic, Operands, MnemonicOpsEndInd);
-
// FIXME: As said above, this is all a pretty gross hack. This instruction
// does not fit with other "subs" and tblgen.
// Adjust operands of B9.3.19 SUBS PC, LR, #imm (Thumb2) system instruction
diff --git a/llvm/test/MC/ARM/load-store-acquire-release-v8-thumb.s b/llvm/test/MC/ARM/load-store-acquire-release-v8-thumb.s
index be8d3c324e6823..b802bfdb28f7de 100644
--- a/llvm/test/MC/ARM/load-store-acquire-release-v8-thumb.s
+++ b/llvm/test/MC/ARM/load-store-acquire-release-v8-thumb.s
@@ -14,6 +14,10 @@
@ CHECK-V7: error: instruction requires: acquire/release
@ CHECK-V7: error: instruction requires: acquire/release
+@ GNU alias
+ ldaexd r6, [r8]
+@ CHECK: ldaexd r6, r7, [r8] @ encoding: [0xd8,0xe8,0xff,0x67]
+
stlexb r1, r3, [r4]
stlexh r4, r2, [r5]
stlex r2, r1, [r7]
@@ -27,6 +31,10 @@
@ CHECK-V7: error: instruction requires: acquire/release
@ CHECK-V7: error: instruction requires: acquire/release
+@ GNU alias
+ stlexd r6, r2, [r8]
+@ CHECK: stlexd r6, r2, r3, [r8] @ encoding: [0xc8,0xe8,0xf6,0x23]
+
lda r5, [r6]
ldab r5, [r6]
ldah r12, [r9]
diff --git a/llvm/test/MC/ARM/load-store-acquire-release-v8.s b/llvm/test/MC/ARM/load-store-acquire-release-v8.s
index 273519e050b1c5..edfe14c93e4ead 100644
--- a/llvm/test/MC/ARM/load-store-acquire-release-v8.s
+++ b/llvm/test/MC/ARM/load-store-acquire-release-v8.s
@@ -1,4 +1,5 @@
-@ RUN: llvm-mc -triple=armv8 -show-encoding < %s | FileCheck %s
+@ RUN: not llvm-mc -triple=armv8 -show-encoding < %s 2> %t | FileCheck %s
+@ RUN: FileCheck %s < %t --check-prefix=CHECK-ERROR
@ RUN: not llvm-mc -triple=armv7 -show-encoding < %s 2>&1 | FileCheck %s --check-prefix=CHECK-V7
ldaexb r3, [r4]
ldaexh r2, [r5]
@@ -14,6 +15,13 @@
@ CHECK-V7: instruction requires: acquire/release
@ CHECK-V7: instruction requires: acquire/release
+ ldaexd r2, r4, [r8]
+@ CHECK-ERROR: error: destination operands must be sequential
+
+@ GNU alias
+ ldaexd r6, [r8]
+@ CHECK: ldaexd r6, r7, [r8] @ encoding: [0x9f,0x6e,0xb8,0xe1]
+
stlexb r1, r3, [r4]
stlexh r4, r2, [r5]
stlex r2, r1, [r7]
@@ -27,6 +35,13 @@
@ CHECK-V7: instruction requires: acquire/release
@ CHECK-V7: instruction requires: acquire/release
+ stlexd r6, r2, r4, [r8]
+@ CHECK-ERROR: error: source operands must be sequential
+
+@ GNU alias
+ stlexd r6, r2, [r8]
+@ CHECK: stlexd r6, r2, r3, [r8] @ encoding: [0x92,0x6e,0xa8,0xe1]
+
lda r5, [r6]
ldab r5, [r6]
ldah r12, [r9]
More information about the llvm-commits
mailing list