[PATCH] [ARM64] Enable feature predicates for NEON / FP / CRYPTO.

Kevin Qin kevinqindev at gmail.com
Wed Apr 16 21:30:05 PDT 2014


  See comments below.


================
Comment at: lib/Target/ARM64/ARM64CallingConvention.td:50
@@ -49,3 +49,3 @@
                                    [Q0, Q1, Q2, Q3, Q4, Q5, Q6, Q7]>>,
-  CCIfType<[v2i64, v4i32, v8i16, v16i8, v4f32, v2f64],
+  CCIfType<[f128, v2i64, v4i32, v8i16, v16i8, v4f32, v2f64],
            CCAssignToReg<[Q0, Q1, Q2, Q3, Q4, Q5, Q6, Q7]>>,
----------------
Tim Northover wrote:
> Doesn't the first change make this unnecessary? I'm confused.
The reason is,  f128 is fp used value type, and v2i64 is neon used value type. If user configured backend by '-mattr=+fp-armv8,-neon', then f128 will bitcast to v2i64. Because v2i64 is not legal value type, it will trigger some split functions falling to an assert. Actually, I also confused about calling convention about v2f64, v3f32. They are bit converted to type v2i64 first, and also directly assgin to Qx registers. I don't think v2f64, v4f32 are different from other vectors like v4i32, so the line "CCIfType<[v2f64, v4f32, f128], CCBitConvertToType<v2i64>>," is unnecessary I think.

================
Comment at: lib/Target/ARM64/ARM64ISelLowering.cpp:93-94
@@ +92,4 @@
+    addRegisterClass(MVT::f128, &ARM64::FPR128RegClass);
+    addRegisterClass(MVT::v16i8, &ARM64::FPR8RegClass);
+    addRegisterClass(MVT::v8i16, &ARM64::FPR16RegClass);
+  }
----------------
Tim Northover wrote:
> I *think* these lines are for the SISD support, which probably means they should be under hasNEON. (Actually, I'm rather confused about their entire existence, but that's the only explanation I can come up with).
I think it's not a good implementation for SISD instructions using v16i8 and v8i16. I will move them to hasNEON part in this patch, but I suggest to refactor this in future.


http://reviews.llvm.org/D3396






More information about the llvm-commits mailing list