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

Tim Northover t.p.northover at gmail.com
Tue Sep 9 11:35:40 PDT 2014


Hi Renato,

The names look reasonable. About the one thing I'd change is not mentioning "Operands" in ARMAddressingModes.h. That's entirely a bike-shedding point though, I wouldn't have mentioned it if you hadn't specifically mentioned names as something you weren't sure of.

Cheers.

Tim.

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:1653-1654
@@ +1652,4 @@
+  bool isNEONi32splatNot() const {
+    if (isNEONByteReplicate(4))
+      return false; // Leave that for bytes replication and forbid by default.
+    if (!isImm())
----------------
Is this right? I don't know what it's trying to exclude for VBIC, but I'd be slightly surprised if just these 4 aliases also make it work for VAND too.

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:2434-2435
@@ +2433,4 @@
+    const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(getImm());
+    unsigned Value = CE->getValue();
+    Value = ~Value & 0xffff;
+    Value = ARM_AM::encodeNEONi16splatOperands(Value);
----------------
It'd be nice if there was some consistency between the i16 and i32 variants here (int/int64_t/unsigned/...). It feels like there ought to be a good canonical representation.

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h:581
@@ +580,3 @@
+    if (Value >= 256)
+      Value = (Value >> 8) | 0xa00;
+    else
----------------
assert on low bits here, when you get around to fixing the bug?

http://reviews.llvm.org/D5263






More information about the llvm-commits mailing list