Skip to content

[MIPS] Fix error messages when rejecting certain assembly not supported by ISA #94695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 2, 2024
Merged

[MIPS] Fix error messages when rejecting certain assembly not supported by ISA #94695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be78400
[MIPS] Fix wrong errors being emitted when rejecting certain assembly…
jdeguire Jun 6, 2024
a480995
Update based on comments from LLVM PR 94695.
jdeguire Aug 22, 2024
5bc2c05
Fix formatting issue flagged by clang-format.
jdeguire Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6410,7 +6410,12 @@ bool MipsAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {

// Check if the current operand has a custom associated parser, if so, try to
// custom parse the operand, or fallback to the general approach.
ParseStatus Res = MatchOperandParserImpl(Operands, Mnemonic);
// Setting the third parameter to true tells the parser to keep parsing even
// if the operands are not supported with the current feature set. In this
// case, the instruction matcher will output a "instruction requires a CPU
// feature not currently enabled" error. If this were false, the parser would
// stop here and output a less useful "invalid operand" error.
ParseStatus Res = MatchOperandParserImpl(Operands, Mnemonic, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not very clear about the purpose of the third parameter ParseForAllFeatures. Some other architectures do not assign a value to this parameter.

Copy link
Contributor Author

@jdeguire jdeguire Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, setting the third parameter to True tells the operand parser to accept the operands if they are not valid with the current feature set, but would be valid if the correct feature set were enabled. An example of this would be an instruction that could take floating-point registers if the target device had an FPU. Finding the exact instruction variant that takes those operands is done after parsing them and it is at that point the "instruction requires a CPU feature not currently enabled" error is triggered. The default for that third parameter is False, which in this case causes the operand parser to give up if an operand is not valid in the current feature set. The symptom is that you will receive a less useful "invalid operand" error instead of one telling you that you do not have the correct features enabled.

In these MIPS tests, I think what was happening is that the instructions themselves required certain MIPS feature levels and that transitively applies to their operands, thus giving the incorrect error about the operand being invalid when in fact the problem is that the whole instruction needs a feature not currently enabled. I can't state this for sure because I'm not very familiar with what TableGen is doing. I actually happened to find this while working on MIPS16 stuff and only realized later that my change here also seemed to help these "wrong-error" tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment here to explain what the third parameter does. Hopefully that is helpful.

if (Res.isSuccess())
return false;
// If there wasn't a custom match, try the generic matcher below. Otherwise,
Expand Down
10 changes: 6 additions & 4 deletions llvm/test/MC/Mips/cnmips/invalid-wrong-error.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# These were correctly rejected as not being supported but the wrong
# error message was emitted.
# RUN: not llvm-mc %s -triple=mips64-unknown-linux -show-encoding -mcpu=octeon 2>%t1
# RUN: FileCheck %s < %t1

.set noat
lwc3 $4, 0($5) # CHECK: :{{[0-9]+}}:{{[0-9]+}}: error: invalid operand for instruction
swc3 $4, 0($5) # CHECK: :{{[0-9]+}}:{{[0-9]+}}: error: invalid operand for instruction
ldc3 $4, 0($5) # CHECK: :{{[0-9]+}}:{{[0-9]+}}: error: invalid operand for instruction
sdc3 $4, 0($5) # CHECK: :{{[0-9]+}}:{{[0-9]+}}: error: invalid operand for instruction
lwc3 $4, 0($5) # CHECK: :[[#]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swc3 $4, 0($5) # CHECK: :[[#]]:[[#]]: error: instruction requires a CPU feature not currently enabled
ldc3 $4, 0($5) # CHECK: :[[#]]:[[#]]: error: instruction requires a CPU feature not currently enabled
sdc3 $4, 0($5) # CHECK: :[[#]]:[[#]]: error: instruction requires a CPU feature not currently enabled
100 changes: 50 additions & 50 deletions llvm/test/MC/Mips/eva/invalid-noeva-wrong-error.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# invalid operand for instructions that are invalid without -mattr=+eva flag and
# are correctly rejected but use the wrong error message at the moment.
# Instructions that are invalid without -mattr=+eva flag. These were rejected
# correctly but used to emit an incorrect error message.
#
# RUN: not llvm-mc %s -triple=mips64-unknown-linux -show-encoding -mcpu=mips32r2 2>%t1
# RUN: FileCheck %s < %t1
Expand All @@ -19,51 +19,51 @@
# RUN: FileCheck %s < %t1

.set noat
cachee 31, 255($7) # CHECK: :[[@LINE]]:23: error: invalid operand for instruction
cachee 0, -256($4) # CHECK: :[[@LINE]]:22: error: invalid operand for instruction
cachee 5, -140($4) # CHECK: :[[@LINE]]:22: error: invalid operand for instruction
lbe $10,-256($25) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lbe $13,255($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lbe $11,146($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lbue $13,-256($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lbue $13,255($v0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lbue $13,-190($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhe $13,-256($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhe $12,255($s0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhe $13,81($s0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhue $s2,-256($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhue $s2,255($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhue $s6,-168($v0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lle $v0,-256($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lle $v1,255($s3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lle $v1,-71($s6) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwe $15,255($a2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwe $13,-256($a2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwe $15,-200($a1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwle $s6,255($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwle $s7,-256($10) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwle $s7,-176($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwre $zero,255($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwre $zero,-256($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwre $zero,-176($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
prefe 14, -256($2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
prefe 11, 255($3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
prefe 14, -37($3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
sbe $s1,255($11) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
sbe $s1,-256($10) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
sbe $s3,0($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
sce $9,255($s2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
sce $12,-256($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
sce $13,-31($s7) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
she $14,255($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
she $14,-256($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
she $9,235($11) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swe $ra,255($sp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swe $ra,-256($sp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swe $ra,-53($sp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swle $9,255($s1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swle $10,-256($s3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swle $8,131($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swre $s4,255($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swre $s4,-256($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swre $s2,86($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
cachee 31, 255($7) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
cachee 0, -256($4) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
cachee 5, -140($4) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lbe $10,-256($25) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lbe $13,255($15) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lbe $11,146($14) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lbue $13,-256($v1) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lbue $13,255($v0) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lbue $13,-190($v1) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lhe $13,-256($s5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lhe $12,255($s0) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lhe $13,81($s0) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lhue $s2,-256($v1) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lhue $s2,255($v1) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lhue $s6,-168($v0) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lle $v0,-256($s5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lle $v1,255($s3) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lle $v1,-71($s6) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwe $15,255($a2) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwe $13,-256($a2) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwe $15,-200($a1) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwle $s6,255($15) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwle $s7,-256($10) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwle $s7,-176($13) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwre $zero,255($gp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwre $zero,-256($gp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwre $zero,-176($gp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
prefe 14, -256($2) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
prefe 11, 255($3) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
prefe 14, -37($3) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
sbe $s1,255($11) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
sbe $s1,-256($10) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
sbe $s3,0($14) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
sce $9,255($s2) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
sce $12,-256($s5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
sce $13,-31($s7) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
she $14,255($15) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
she $14,-256($15) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
she $9,235($11) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swe $ra,255($sp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swe $ra,-256($sp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swe $ra,-53($sp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swle $9,255($s1) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swle $10,-256($s3) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swle $8,131($s5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swre $s4,255($13) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swre $s4,-256($13) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swre $s2,86($14) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
24 changes: 12 additions & 12 deletions llvm/test/MC/Mips/eva/invalid_R6.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@
# RUN: FileCheck %s < %t1

.set noat
lwle $s6,255($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwle $s7,-256($10) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwle $s7,-176($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwre $zero,255($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwre $zero,-256($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwre $zero,-176($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swle $9,255($s1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swle $10,-256($s3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swle $8,131($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swre $s4,255($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swre $s4,-256($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
swre $s2,86($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lwle $s6,255($15) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwle $s7,-256($10) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwle $s7,-176($13) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwre $zero,255($gp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwre $zero,-256($gp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwre $zero,-176($gp) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swle $9,255($s1) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swle $10,-256($s3) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swle $8,131($s5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swre $s4,255($13) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swre $s4,-256($13) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swre $s2,86($14) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lle $33, 8($5) # CHECK: :[[@LINE]]:19: error: invalid register number
lle $4, 8($33) # CHECK: :[[@LINE]]:25: error: invalid register number
lle $4, 512($5) # CHECK: :[[@LINE]]:23: error: expected memory with 9-bit signed offset
Expand Down
11 changes: 6 additions & 5 deletions llvm/test/MC/Mips/micromips32r6/invalid-wrong-error.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Instructions that are correctly rejected but emit a wrong or misleading error.
# Instructions that were correctly rejected but used to emit a wrong or
# misleading error.
# RUN: not llvm-mc %s -triple=mips -show-encoding -mcpu=mips32r6 -mattr=micromips 2>%t1
# RUN: FileCheck %s < %t1

Expand Down Expand Up @@ -28,7 +29,7 @@
sc $4, -513($5) # CHECK: :[[@LINE]]:3: error: instruction requires a CPU feature not currently enabled
ll $4, 512($5) # CHECK: :[[@LINE]]:3: error: instruction requires a CPU feature not currently enabled
ll $4, -513($5) # CHECK: :[[@LINE]]:3: error: instruction requires a CPU feature not currently enabled
lwr $4, 1($5) # CHECK: :[[@LINE]]:11: error: invalid operand for instruction
lwl $4, 1($5) # CHECK: :[[@LINE]]:11: error: invalid operand for instruction
swr $4, 1($5) # CHECK: :[[@LINE]]:11: error: invalid operand for instruction
swl $4, 1($5) # CHECK: :[[@LINE]]:11: error: invalid operand for instruction
lwr $4, 1($5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
lwl $4, 1($5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swr $4, 1($5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
swl $4, 1($5) # CHECK: :[[#@LINE]]:[[#]]: error: instruction requires a CPU feature not currently enabled
Loading
Loading