[PATCH] D143406: [MIPS] Asm: Improved diagnostics when a memory operand and unsupported CPU feature are involved

[Davide Mor via Phabricator via llvm-commits]

Tazdevil971 added a comment.

Yes and no, not all targets benefit from this the same way. Also it doesn't always work as intended, if you look at file llvm/test/MC/Mips/mips2/invalid-mips3-wrong-error.s, there is a case where it actually gives you worst diagnostics.

It all depends on the target and how good the fallback parser is, in the case of MIPS it's pretty bad, as it only handles immediates and registers (thus the old "wrong operand" error, the fallback parser was incorrectly guessing the kind of operand, immediate instead of memory). As far as I understand the issue was similar with AArch64 as SVE added a lot of complexity to correctly guessing the kind of operand so they resorted to this solution.

I think it's more of a case by case thing, and we should review each case individually.



================
Comment at: llvm/test/MC/Mips/cnmips/invalid.s:28
+    sdc3  $4, 0($5)      # CHECK: :[[@LINE]]:5: error: instruction requires a CPU feature not currently enabled
\ No newline at end of file

----------------
jrtc27 wrote:
> Please fix these
Sorry, what's wrong about them? These are COP3 instructions which are disabled when cnmips is enabled, the message is kind of correct, these instructions require a different set of CPU features. 

Or do you mean the file endings?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143406/new/

https://reviews.llvm.org/D143406