[PATCH] ARM: Implement big endian bit-conversion for NEON types

Christian Pirker cpirker at a-bix.com
Wed May 7 10:10:30 PDT 2014

Hi James,

Thanks for your feedback.

> It doesn't look like this patch does anything for argument passing or lowering formal arguments. Is this going to follow in another patch or do you think there's nothing to do here?

The patch converts vector arguments into integer format (and vice versa) when passing vectors as function arguments, as required by the ABI.


Comment at: lib/Target/ARM/ARMFastISel.cpp:192
@@ -191,1 +191,3 @@
+    const TargetLowering* getTargetLowering() { return TM.getTargetLowering(); }
James Molloy wrote:
> This function seems unused?
This function is required to evaluate the [IsBE] predicate for the bitconversion rules that specify the VREV instructions.

Comment at: lib/Target/ARM/ARMISelLowering.cpp:3827
@@ -3826,1 +3826,3 @@
   if (DstVT == MVT::i64 && TLI.isTypeLegal(SrcVT)) {
+    SDValue OpSrc = TLI.isBigEndian() && SrcVT.isVector() ?
+                      DAG.getNode(ARMISD::VREV64, dl, SrcVT, Op) : Op;
James Molloy wrote:
> This seems too complex for a ternary - I'd prefer to see it written as an if/else.
Yes, if/else would be more readable.

Comment at: lib/Target/ARM/ARMInstrNEON.td:2369
@@ -2368,3 +2368,3 @@
 def : Pat<(v2f64 (word_alignedload addrmode6:$addr)),
-          (VLD1q32 addrmode6:$addr)>;
+          (VLD1q32 addrmode6:$addr)>, Requires<[IsLE]>;
 def : Pat<(word_alignedstore (v2f64 QPR:$value), addrmode6:$addr),
James Molloy wrote:
> Why are these not allowed in big-endian mode? and why only these patterns?
v2f64 is not compatible with v4i32 in big endian mode (a vrev instruction would be needed). All other patterns like v8i8,... are already disabled in big endian mode. Not sure why this specific rule was left active for both endian modes.

Comment at: lib/Target/ARM/ARMInstrNEON.td:6261
@@ -6240,1 +6260,3 @@
+let Predicates = [IsBE] in {
+  // 64 bit conversions
James Molloy wrote:
> These look fine, but what was your methodology for generating them? Did you do them all by hand, or use some script?
I did it by hand, based on similar patch for AArch64 (D3424).


More information about the llvm-commits mailing list