[ARM64EC] Warn on using disallowed registers in assembly src. #93618
Conversation
ARM64EC designates a set of disallowed registers, because a mapping does not exist from them to x64. The MSVC assembler (armasm64) has a warning for this. A test is also included as part of the patch. See the list of disallowed registers below: https://learn.microsoft.com/en-us/cpp/build/arm64ec-windows-abi-conventions?view=msvc-170#register-mapping
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-mc Author: Namish Kukreja (namikukr) ChangesARM64EC designates a set of disallowed registers, because a mapping does not exist from them to x64. The MSVC assembler (armasm64) has a warning for this. A test is also included as part of the patch. See the list of disallowed registers below: Full diff: https://github.com/llvm/llvm-project/pull/93618.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 13a68b7dcf984..ea329dd71e398 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -294,11 +294,13 @@ class AArch64AsmParser : public MCTargetAsmParser {
#include "AArch64GenAsmMatcher.inc"
};
bool IsILP32;
+ bool IsWindowsArm64EC;
AArch64AsmParser(const MCSubtargetInfo &STI, MCAsmParser &Parser,
const MCInstrInfo &MII, const MCTargetOptions &Options)
: MCTargetAsmParser(Options, STI, MII) {
IsILP32 = STI.getTargetTriple().getEnvironment() == Triple::GNUILP32;
+ IsWindowsArm64EC = STI.getTargetTriple().isWindowsArm64EC();
MCAsmParserExtension::Initialize(Parser);
MCStreamer &S = getParser().getStreamer();
if (S.getTargetStreamer() == nullptr)
@@ -5315,6 +5317,31 @@ bool AArch64AsmParser::validateInstruction(MCInst &Inst, SMLoc &IDLoc,
}
}
+ // On ARM64EC, only valid registers may be used. Warn against using
+ // explicitly disallowed registers.
+ if (IsWindowsArm64EC) {
+ for (unsigned i = 0; i < Inst.getNumOperands(); ++i) {
+ if (Inst.getOperand(i).isReg()) {
+ unsigned Reg = Inst.getOperand(i).getReg();
+ // At this point, vector registers are matched to their
+ // appropriately sized alias.
+ if ((Reg == AArch64::W13 || Reg == AArch64::X13) ||
+ (Reg == AArch64::W14 || Reg == AArch64::X14) ||
+ (Reg == AArch64::W23 || Reg == AArch64::X23) ||
+ (Reg == AArch64::W24 || Reg == AArch64::X24) ||
+ (Reg == AArch64::W28 || Reg == AArch64::X28) ||
+ (Reg >= AArch64::Q16 && Reg <= AArch64::Q31) ||
+ (Reg >= AArch64::D16 && Reg <= AArch64::D31) ||
+ (Reg >= AArch64::S16 && Reg <= AArch64::S31) ||
+ (Reg >= AArch64::H16 && Reg <= AArch64::H31) ||
+ (Reg >= AArch64::B16 && Reg <= AArch64::B31)) {
+ Warning(IDLoc, "this instruction uses disallowed registers.");
+ break;
+ }
+ }
+ }
+ }
+
// Check for indexed addressing modes w/ the base register being the
// same as a destination/source register or pair load where
// the Rt == Rt2. All of those are undefined behaviour.
diff --git a/llvm/test/MC/AArch64/arm64ec-disallowed-regs.s b/llvm/test/MC/AArch64/arm64ec-disallowed-regs.s
new file mode 100644
index 0000000000000..8061987a51998
--- /dev/null
+++ b/llvm/test/MC/AArch64/arm64ec-disallowed-regs.s
@@ -0,0 +1,75 @@
+// RUN: llvm-mc -triple arm64ec-pc-windows-msvc < %s 2> %t.log
+// RUN: FileCheck %s --check-prefix=CHECK-ERR < %t.log
+
+
+// ---- disallowed x registers ----
+orr x13, x0, x1 // x13
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr x14, x2, x3 // x14
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr x4, x23, x5 // x23
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr x6, x7, x24 // x24
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr x28, x8, x9 // x28
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+
+// ---- disallowed w registers ----
+orr w0, w13, w1 // w13
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr w14, w2, w3 // w14
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr w4, w23, w5 // w23
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr w6, w7, w24 // w24
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orr w28, w8, w9 // w28
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+
+// ---- disallowed vector registers ----
+orn v1.8b, v16.8b, v2.8b // v16
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v2.16b, v17.16b, v3.16b // v17
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v3.8b, v18.8b, v4.8b // v18
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v4.16b, v19.16b, v5.16b // v19
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v5.8b, v20.8b, v6.8b // v20
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v21.8b, v6.8b, v7.8b // v21
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v7.16b, v8.16b, v22.16b // v22
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v23.8b, v8.8b, v9.8b // v23
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v9.16b, v24.16b, v10.16b // v24
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v10.8b, v25.8b, v11.8b // v25
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v11.8b, v12.8b, v26.8b // v26
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v12.8b, v27.8b, v13.8b // v27
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v13.16b, v28.16b, v14.16b // v28
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v14.8b, v29.8b, v15.8b // v29
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v15.8b, v30.8b, v15.8b // v30
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+orn v1.16b, v31.16b, v1.16b // v31
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+
+// ---- random tests on h, b, d, s registers ----
+orn.16b v1, v16, v2
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+mov.4h v17, v8
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+fmul.2s v2, v18, v11
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+clz.8h v3, v19
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+add.4s v0, v20, v1
+// CHECK-ERR: warning: this instruction uses disallowed registers.
+add.2d v0, v20, v1
+// CHECK-ERR: warning: this instruction uses disallowed registers.
\ No newline at end of file
|
efriedma-quic
requested review from
mstorsjo,
cjacek,
dpaoliello and
efriedma-quic
| add.4s v0, v20, v1 | ||
| // CHECK-ERR: warning: this instruction uses disallowed registers. | ||
| add.2d v0, v20, v1 | ||
| // CHECK-ERR: warning: this instruction uses disallowed registers. |
There was a problem hiding this comment.
Nit: The file seems to be lacking a trailing end of line character
When assembling with |
But you mentioned |
I apologize - I'm a bit new to this so I'm just getting familiar. That seems accurate. I'm working on making the changes that were suggested and will be back with an update as soon as I have one. |
yes, armasm64 automatically enables this warning when target ARM64EC, the flag is |
| // CHECK-ERR: warning: register Q20 is disallowed on ARM64EC. | ||
| add.2d v0, v20, v1 | ||
| // CHECK-ERR: warning: register Q20 is disallowed on ARM64EC. | ||
|
|
There was a problem hiding this comment.
you may want to try data width less than DoubleWord (D, 64-bit), e.g. 4b, 2h, s and see if the warning msg still uses Dx register name or S/H/B names.
Also worth to try some instructions like fadd s0, s1, s16 and see what reg names is emmitted with the warning msg.
| (Reg >= AArch64::H16 && Reg <= AArch64::H31) || | ||
| (Reg >= AArch64::B16 && Reg <= AArch64::B31)) { | ||
| Warning(IDLoc, "register " + Twine(RI->getName(Reg)) + | ||
| " is disallowed on ARM64EC."); |
There was a problem hiding this comment.
now it warns on each disallowed register, you can add an orr v1, v16, v17 to the tests and check that two warnings are issued for this instruction
| // ---- random tests on h, b, d, s registers ---- | ||
| orn.16b v1, v16, v2 | ||
| // CHECK-ERR: warning: register Q16 is disallowed on ARM64EC. | ||
| mov.4h v17, v8 |
There was a problem hiding this comment.
Instead of using these nonstandard(?) assembler forms with mov.4h, can't we use instructions that natively use the form d17? E.g. str d17, [x0]?
There was a problem hiding this comment.
I just pushed a patch that hopefully fixes this, let me know if you have more concerns!
…ple disallowed registers
|
No further comments from me here, this looks good now, but I'll wait for others who also have commented, before approving. |
|
gentle ping... |
|
@namikukr Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
ARM64EC designates a set of disallowed registers, because a mapping does not exist from them to x64. The MSVC assembler (armasm64) has a warning for this.
A test is also included as part of the patch.
See the list of disallowed registers below:
https://learn.microsoft.com/en-us/cpp/build/arm64ec-windows-abi-conventions?view=msvc-170#register-mapping