[ARM] Validate STREX instruction in the assembler #85074
Conversation
|
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-arm @llvm/pr-subscribers-mc Author: None (adamszilagyi) ChangesBased on the ARM Architecture Reference Manual: Full diff: https://github.com/llvm/llvm-project/pull/85074.diff 2 Files Affected:
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index c320bf723c88bb..8c96cce7a5cf55 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -7629,6 +7629,39 @@ bool ARMAsmParser::validateInstruction(MCInst &Inst,
}
return false;
}
+ case ARM::t2STREXB:
+ case ARM::t2STREXH:
+ case ARM::t2STREX:
+ case ARM::STREXB:
+ case ARM::STREXH:
+ case ARM::STREX:
+ case ARM::STREXD: {
+ unsigned Rd = MRI->getEncodingValue(Inst.getOperand(0).getReg());
+ unsigned Rm = MRI->getEncodingValue(Inst.getOperand(1).getReg());
+ unsigned Rn = MRI->getEncodingValue(Inst.getOperand(2).getReg());
+
+ if (Rd == 15 || Rm == 15 || Rn == 15)
+ return Error(Operands[3]->getStartLoc(), "operand can't be R15");
+
+ if (Rd == Rm || Rd == Rn || (Opcode == ARM::STREXD && Rd == Rm + 1))
+ return Error(Operands[3]->getStartLoc(),
+ "destination operand can't be identical to source operand");
+ return false;
+ }
+ case ARM::t2STREXD: {
+ unsigned Rd = MRI->getEncodingValue(Inst.getOperand(0).getReg());
+ unsigned Rm1 = MRI->getEncodingValue(Inst.getOperand(1).getReg());
+ unsigned Rm2 = MRI->getEncodingValue(Inst.getOperand(2).getReg());
+ unsigned Rn = MRI->getEncodingValue(Inst.getOperand(3).getReg());
+
+ if (Rd == 15 || Rn == 15)
+ return Error(Operands[4]->getStartLoc(), "operand can't be R15");
+
+ if (Rd == Rm1 || Rd == Rm2 || Rd == Rn)
+ return Error(Operands[4]->getStartLoc(),
+ "destination operand can't be identical to source operand");
+ return false;
+ }
case ARM::t2IT: {
// Encoding is unpredictable if it ever results in a notional 'NV'
// predicate. Since we don't parse 'NV' directly this means an 'AL'
diff --git a/llvm/test/MC/ARM/diagnostics.s b/llvm/test/MC/ARM/diagnostics.s
index e6d80ea7a6280c..f29be4e3bafcc8 100644
--- a/llvm/test/MC/ARM/diagnostics.s
+++ b/llvm/test/MC/ARM/diagnostics.s
@@ -336,6 +336,58 @@
@ CHECK-ERRORS: strexd r6, r5, r3, [r8]
@ CHECK-ERRORS: ^
+ @ Invalid Rm/Rn operands for strex
+ strex r1, r2, [r1]
+ strex r1, r1, [r2]
+
+@ CHECK-ERRORS: error: destination operand can't be identical to source operand
+@ CHECK-ERRORS: strex r1, r2, [r1]
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: destination operand can't be identical to source operand
+@ CHECK-ERRORS: strex r1, r1, [r2]
+@ CHECK-ERRORS: ^
+
+ @ Can not use R15 as operand for strex
+ strex r15, r2, [r1]
+ strex r1, r15, [r2]
+ strex r1, r2, [r15]
+
+@ CHECK-ERRORS: error: operand can't be R15
+@ CHECK-ERRORS: strex r15, r2, [r1]
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: operand can't be R15
+@ CHECK-ERRORS: strex r1, r15, [r2]
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: operand can't be R15
+@ CHECK-ERRORS: strex r1, r2, [r15]
+@ CHECK-ERRORS: ^
+
+ @ Can not use R15 as operand for strexd
+ strexd r15, r0, r1, [r6]
+ strexd r2, r0, r1, [r15]
+
+@ CHECK-ERRORS: error: operand can't be R15
+@ CHECK-ERRORS: strexd r15, r0, r1, [r6]
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: operand can't be R15
+@ CHECK-ERRORS: strexd r2, r0, r1, [r15]
+@ CHECK-ERRORS: ^
+
+ @ Invalid Rm/Rn operands for strexd
+ strexd r1, r0, r1, [r6]
+ strexd r2, r2, r3, [r6]
+ strexd r4, r0, r1, [r4]
+
+@ CHECK-ERRORS: error: destination operand can't be identical to source operand
+@ CHECK-ERRORS: strexd r1, r0, r1, [r6]
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: destination operand can't be identical to source operand
+@ CHECK-ERRORS: strexd r2, r2, r3, [r6]
+@ CHECK-ERRORS: ^
+@ CHECK-ERRORS: error: destination operand can't be identical to source operand
+@ CHECK-ERRORS: strexd r4, r0, r1, [r4]
+@ CHECK-ERRORS: ^
+
@ Illegal rotate operators for extend instructions
sxtb r8, r3, #8
sxtb r8, r3, ror 24
|
davemgreen
requested review from
ostannard,
statham-arm,
efriedma-quic and
tmatheson-arm
|
|
||
| @ CHECK-ERRORS: error: operand can't be R15 | ||
| @ CHECK-ERRORS: strex r15, r2, [r1] | ||
| @ CHECK-ERRORS: ^ |
There was a problem hiding this comment.
It would be better if this pointed to the problematic operand (same for the cases below too). Especially if someone writes pc rather than r15, the error message would be confusing.
| unsigned Rn = MRI->getEncodingValue(Inst.getOperand(2).getReg()); | ||
|
|
||
| if (Rd == 15 || Rm == 15 || Rn == 15) | ||
| return Error(Operands[3]->getStartLoc(), "operand can't be R15"); |
There was a problem hiding this comment.
I'm not sure we handle other cases of UNPREDICTABLE, but it seems like these should be warnings rather than errors, since it is possible to actually emit the encoding.
Based on the ARM Architecture Reference Manual:
Use of R15
Specifying R15 for register <Rd>, <Rn>, or <Rm> has UNPREDICTABLE results.Operand restrictions
<Rd> must be distinct from both <Rm> and <Rn>, otherwise the results are UNPREDICTABLE.Altough in codegen it is not possible to end up with a
strexinstruction with same register as destination and source, because the regallocator is restricted by the instructions definion where the destination register is marked as@earlyclobber, with inline asm it is possible to overcome this and write a code like the followingwhich previously went through the assembler whitout any diagnostic that the behaviour of the
strexis unpredictable in this case.