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

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
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;
----------------
SjoerdMeijer wrote:
> rengolin wrote:
> > Why is this one different? Why does it need the check inline, unlike the others?
> Thanks for reviewing. There are quite some subtleties here. For example, some instructions encode  immediate values as "value - 1", and then it needs to print them out as "value + 1". I believe this was one these cases, and using the AsmOperand class here messes up that logic a bit. I fully agree that it would be nicer and more consistent to have a similar approach here. But as you also wrote in the other remark, I would like to address these things in follow up commits as this commit is already quite large; thought this would be a good point to stop as a first clean up.
I'm happy for these cleanups to come in following patches.


https://reviews.llvm.org/D31333