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

James Molloy james.molloy at arm.com
Wed May 7 08:24:34 PDT 2014


Hi Christian,

Generally this looks good and I have no real problems with it, just a few small nits.

> The patch covers the conversion between floating point and vector types, and the function argument passing of vector types providing compatibility to the ARM ABI.

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?

Cheers,

James

================
Comment at: lib/Target/ARM/ARMFastISel.cpp:192
@@ -191,1 +191,3 @@
 
+    const TargetLowering* getTargetLowering() { return TM.getTargetLowering(); }
+
----------------
This function seems unused?

================
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;
----------------
This seems too complex for a ternary - I'd prefer to see it written as an if/else.

================
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),
----------------
Why are these not allowed in big-endian mode? and why only these patterns?

================
Comment at: lib/Target/ARM/ARMInstrNEON.td:6261
@@ -6240,1 +6260,3 @@
 
+let Predicates = [IsBE] in {
+  // 64 bit conversions
----------------
These look fine, but what was your methodology for generating them? Did you do them all by hand, or use some script?

http://reviews.llvm.org/D3651






More information about the llvm-commits mailing list