[PATCH] D31333: ARMAsmParser: clean up of isImmediate functions

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Hi Sjord,

Nice cleanup! Thanks!

I have a few general inline remarks, but overall, the patch looks good. I'll let Oliver finish his review, and I'm happy when he is.

cheers,
--renato



================
Comment at: lib/Target/ARM/ARMInstrInfo.td:694
+def Imm8_255AsmOperand: ImmAsmOperand<8,255> { let Name = "Imm8_255"; }
+def imm8_255 : Operand<i32>, ImmLeaf<i32, [{
+  return Imm >= 8 && Imm < 256;
----------------
Why is this one different? Why does it need the check inline, unlike the others?


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:950
   }
   bool isImm0_508s4Neg() const {
     if (!isImm()) return false;
----------------
Is it possible to clean up the negs in the same way? Maybe for a different patch, though, as this one is quite big already.


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:9092
+  case Match_ImmRange0_3:
+    return Error(ErrorLoc, "immediate operand must be in the range [0,3]");
+  case Match_ImmRange0_7:
----------------
It's a shame you can't put all those `Match_*` into one match error with different parameters. :(


https://reviews.llvm.org/D31333