[MIPS] Fix error messages when rejecting certain assembly not supported by ISA #94695
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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-mc Author: Jesse D (jdeguire) Changes… instructions. This is a fix I stumbled upon while working on something else. I decided to break it out since it seems like a good "first issue" to submit. I updated the comments in the "wrong error" test files to indicate that the messages are no longer incorrect, but I left the names of the test files alone. I was not sure what to do with those, so I would appreciate thoughts or guidance. Patch is 49.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/94695.diff 19 Files Affected:
diff --git a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
index 076e0a20cb97e..2e313fce56028 100644
--- a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
+++ b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
@@ -6410,7 +6410,7 @@ 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);
+ ParseStatus Res = MatchOperandParserImpl(Operands, Mnemonic, true);
if (Res.isSuccess())
return false;
// If there wasn't a custom match, try the generic matcher below. Otherwise,
diff --git a/llvm/test/MC/Mips/cnmips/invalid-wrong-error.s b/llvm/test/MC/Mips/cnmips/invalid-wrong-error.s
index aa96049c9b7db..b4695431087f0 100644
--- a/llvm/test/MC/Mips/cnmips/invalid-wrong-error.s
+++ b/llvm/test/MC/Mips/cnmips/invalid-wrong-error.s
@@ -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: :{{[0-9]+}}:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swc3 $4, 0($5) # CHECK: :{{[0-9]+}}:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ ldc3 $4, 0($5) # CHECK: :{{[0-9]+}}:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ sdc3 $4, 0($5) # CHECK: :{{[0-9]+}}:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
diff --git a/llvm/test/MC/Mips/eva/invalid-noeva-wrong-error.s b/llvm/test/MC/Mips/eva/invalid-noeva-wrong-error.s
index 3318831b81c1f..14143e6cba6b8 100644
--- a/llvm/test/MC/Mips/eva/invalid-noeva-wrong-error.s
+++ b/llvm/test/MC/Mips/eva/invalid-noeva-wrong-error.s
@@ -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
@@ -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]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ cachee 0, -256($4) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ cachee 5, -140($4) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lbe $10,-256($25) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lbe $13,255($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lbe $11,146($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lbue $13,-256($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lbue $13,255($v0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lbue $13,-190($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lhe $13,-256($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lhe $12,255($s0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lhe $13,81($s0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lhue $s2,-256($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lhue $s2,255($v1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lhue $s6,-168($v0) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lle $v0,-256($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lle $v1,255($s3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lle $v1,-71($s6) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwe $15,255($a2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwe $13,-256($a2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwe $15,-200($a1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwle $s6,255($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwle $s7,-256($10) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwle $s7,-176($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwre $zero,255($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwre $zero,-256($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwre $zero,-176($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ prefe 14, -256($2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ prefe 11, 255($3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ prefe 14, -37($3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ sbe $s1,255($11) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ sbe $s1,-256($10) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ sbe $s3,0($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ sce $9,255($s2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ sce $12,-256($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ sce $13,-31($s7) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ she $14,255($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ she $14,-256($15) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ she $9,235($11) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swe $ra,255($sp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swe $ra,-256($sp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swe $ra,-53($sp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swle $9,255($s1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swle $10,-256($s3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swle $8,131($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swre $s4,255($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swre $s4,-256($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swre $s2,86($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
diff --git a/llvm/test/MC/Mips/eva/invalid_R6.s b/llvm/test/MC/Mips/eva/invalid_R6.s
index 07f882dd56c5a..62d3043b2d3cb 100644
--- a/llvm/test/MC/Mips/eva/invalid_R6.s
+++ b/llvm/test/MC/Mips/eva/invalid_R6.s
@@ -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]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwle $s7,-256($10) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwle $s7,-176($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwre $zero,255($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwre $zero,-256($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwre $zero,-176($gp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swle $9,255($s1) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swle $10,-256($s3) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swle $8,131($s5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swre $s4,255($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swre $s4,-256($13) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swre $s2,86($14) # CHECK: :[[@LINE]]:{{[0-9]+}}: 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
diff --git a/llvm/test/MC/Mips/micromips32r6/invalid-wrong-error.s b/llvm/test/MC/Mips/micromips32r6/invalid-wrong-error.s
index 0995b71628d01..061bb776e7f79 100644
--- a/llvm/test/MC/Mips/micromips32r6/invalid-wrong-error.s
+++ b/llvm/test/MC/Mips/micromips32r6/invalid-wrong-error.s
@@ -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
@@ -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]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ lwl $4, 1($5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swr $4, 1($5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+ swl $4, 1($5) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
diff --git a/llvm/test/MC/Mips/mips1/invalid-mips2-wrong-error.s b/llvm/test/MC/Mips/mips1/invalid-mips2-wrong-error.s
index 1b0bcd4f50bb0..4c15153910dfa 100644
--- a/llvm/test/MC/Mips/mips1/invalid-mips2-wrong-error.s
+++ b/llvm/test/MC/Mips/mips1/invalid-mips2-wrong-error.s
@@ -1,16 +1,16 @@
-# Instructions that are invalid and are correctly rejected but use the wrong
-# error message at the moment.
+# Instructions that are invalid and are ...
[truncated]
|
|
Ping (Should I add people like "brad0" did above? The automatic comment makes it seem like this is done automatically, so I didn't do that.) |
|
Pong |
yingopq
reviewed
Jul 1, 2024
| @@ -6410,7 +6410,7 @@ 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); | |||
| ParseStatus Res = MatchOperandParserImpl(Operands, Mnemonic, true); | |||
There was a problem hiding this comment.
I am not very clear about the purpose of the third parameter ParseForAllFeatures. Some other architectures do not assign a value to this parameter.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I added a comment here to explain what the third parameter does. Hopefully that is helpful.
|
Sorry for replying so late because I'm not very familiar with this area. |
|
No problem, I appreciate your response! |
|
Ping! Apologies for the late check-in. Does my explanation of what the change is doing help? I could try adding a comment to the code if that would be better. |
|
Can we give a more detailed error, such as what feature is not set? |
MaskRay
reviewed
Aug 20, 2024
| 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]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled |
There was a problem hiding this comment.
While updating messages, switch to [[#@LINE]]. [[@LINE]] is deprecated FileCheck syntax. Prefer [[#]] to {{[0-9]+}}
There was a problem hiding this comment.
I updated the messages I modified in the the most recent commit. Should I update the other messages in these files? I can do that here or as another pull request later.
-- Add a comment to MipsAsmParser explaining what the third parameter in the call to 'MatchOperandParseImplr()' does. -- Update the error message checks to use newer syntax. This updates only the messages I had originally modified.
It looks like some other targets do this, so I should be able to add that to MIPS, too. Should something like that be a part of this PR or a later one? |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Ping. |
|
LGTM. |
wzssyqa
changed the title
wzssyqa
changed the title
|
LGTM. |
|
@jdeguire 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 by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. 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. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/2210 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
… instructions.
This is a fix I stumbled upon while working on something else. I decided to break it out since it seems like a good "first issue" to submit. I updated the comments in the "wrong error" test files to indicate that the messages are no longer incorrect, but I left the names of the test files alone. I was not sure what to do with those, so I would appreciate thoughts or guidance.