[PATCH] Add aliases for VAND imm to VBIC ~imm

Renato Golin renato.golin at linaro.org
Tue Sep 9 06:27:47 PDT 2014


================
Comment at: lib/Target/ARM/ARMInstrNEON.td:39
@@ +38,3 @@
+def nImmSplatNotI16 : Operand<i32> {
+  let PrintMethod = "printNEONModImmOperand";
+  let ParserMatchClass = nImmSplatNotI16AsmOperand;
----------------
t.p.northover wrote:
> PrintMethod shouldn't be needed since the aliases aren't canonical.
ok

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:1629
@@ +1628,3 @@
+  bool isNEONi16splatNot() const {
+    return isNEONi16splat();
+  }
----------------
t.p.northover wrote:
> Is this right? From the ARM ARM it looks like the set of constants permitted is not invariant under Imm -> ~Imm.
> 
> Actually, the code above in isNEONi16Splat is wrong too (which is why your test happens to work anyway):
>     $ echo "vbic.i16 d10, #0x03ff" | bin/llvm-mc -triple armv7 -show-inst
>         vbic.i16 d10, #0x300
Oh god, no! Lazy arse, I am. I meant to invert the value first, but got too excited that my test case passed! Sorry.

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:2409
@@ +2408,3 @@
+private:
+  unsigned validateNEONi16splatOperands(unsigned Value) const {
+    if (Value >= 256)
----------------
t.p.northover wrote:
> This isn't really a validation (which implies it might be invalid). Perhaps an "encodeNEON..."; if so, it should probably go into MCTargetDesc with the others.
encode is a better name, yes. Where in MCTargetDesc you're thinking of?

http://reviews.llvm.org/D5263






More information about the llvm-commits mailing list