[PATCH] D74120: [ARM][ASMParser] Refuse equal RdHi/RdLo for s/umlal, smlsl, s/umull, umaal

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added inline comments.


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:7978
+    unsigned RdLo = Inst.getOperand(1).getReg();
+    if(getContext().getRegisterInfo()->isSubRegisterEq(RdHi, RdLo)) {
+      return Error(Loc,
----------------
You should just be able to compare RdHi and RdLo directly here, none of these registers have any sub-registers. Doing it this was doesn't really make sense, because isSubRegisterEq is an asymmetric operation.


================
Comment at: llvm/test/MC/ARM/equal-rdhi-rdlo-diagnostics.s:2
+@ RUN: not llvm-mc -triple thumbv7m-eabi -mattr=+dsp < %s 2> %t
+@ RUN: FileCheck < %t %s
+@ RUN: not llvm-mc -triple armv8 -mattr=+dsp < %s 2> %t
----------------
You can redirect the stderr of the llvm-mc command to stdout with `2>&1`, and pipe it into FIleCheck without needing a temporary file.


================
Comment at: llvm/test/MC/ARM/equal-rdhi-rdlo-diagnostics.s:8
+        smlal   r1, r1, r3, r4
+        @ CHECK: error: unpredictable instruction, RdHi and RdLo must be different
+        smlalbb   r1, r1, r3, r4
----------------
It's better to include the line number (using `@LINE`) of the error message in the check line, so that if one of these fails FileCheck will point to the failing one. The other test in this patch (v8_IT_manual.s) is a good example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74120