[PATCH] D39910: [ARM] Issue an eror when non-general-purpose registers used in address operands (alternative)

[Oliver Stannard via Phabricator via llvm-commits]

olista01 added a comment.

It looks like there are a lot of inaccuracies  here still with regards to accepting r13/sp and r15/pc. Rejecting non-GPRs is obviously an improvement, but I don't think we should be adding diagnostics which give wrong information to the user.

Maybe the best course of action here would be to commit just the part restricting registers to GPR now, leaving the diagnostics as "invalid operand". We can then fix the operand classes precisely a few at a time, adding predicate functions and diagnostics that match.



================
Comment at: lib/Target/ARM/ARMInstrInfo.td:608
+    let Name = "RegShiftedReg";
+    let DiagnosticString = "shifted register operand must use two registers in "
+                           "the range [r0, r15]";
----------------
I've been trying to standardise the grammar of these messages on "operand must be a <type> ..." rather than "<type> operand must be ...", see D36689.


================
Comment at: lib/Target/ARM/ARMInstrInfo.td:609
+    let DiagnosticString = "shifted register operand must use two registers in "
+                           "the range [r0, r15]";
+}
----------------
Neither register can be r15, at least for ADD on v7A and v8A.


================
Comment at: lib/Target/ARM/ARMInstrInfo.td:939
+  let DiagnosticString = "operand must be a memory reference with base and "
+                         "index registers in the range [r0, r15], and an "
+                         "optional shift";
----------------
The index register can't be r15 (at least for ARM LDR on v7A).


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:1157
+    if (Memory.BaseRegNum &&
+        !ARMMCRegisterClasses[ARM::GPRRegClassID].contains(Memory.BaseRegNum))
+      return false;
----------------
There is no code here checking for anything more specific than GPR (other than the TBB/TBH operands), but there are lots of instructions that cannot use sp or pc.


https://reviews.llvm.org/D39910