[PATCH] [PPC64] Add vector quadword add/sub instructions for POWER8

Kit Barton kbarton at ca.ibm.com
Mon Apr 20 08:43:31 PDT 2015


================
Comment at: include/llvm/IR/IntrinsicsPowerPC.td:537
@@ -527,1 +536,3 @@
               Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty], [IntrNoMem]>;
+
+  // Add Extended Quadword
----------------
nemanjai wrote:
> Do we not want these guarded for Power8 Altivec feature? It would seem that we don't want these available in the back end for CPU's that don't have the feature.
We don't have any other guards in this file, as far as I can tell. 
I believe this is fine to define these intrinsics as is, and let other diagnostic messages determine whether they are valid for the target architecture. 

================
Comment at: test/CodeGen/PowerPC/vec_add_sub_quadword.ll:9
@@ +8,3 @@
+       ret <1 x i128> %result
+; CHECK: vadduqm 2, 2, 3
+}
----------------
nemanjai wrote:
> I don't think all the registers are ABI-required registers. Probably ok to use a regex for the other(s). Just making the same comment I received previously and I think it works well. A regex can be something like {{[0-9]+}}.
> 
> It may or may not make sense, but you can check that the value being added is loaded into a register and that that register is used for the add (at least for the immediate). Although I don't know if that's a good idea for commutative operations. Maybe someone more knowledgeable can comment on that.
Agreed. Will change. 

================
Comment at: test/CodeGen/PowerPC/vec_add_sub_quadword.ll:15
@@ +14,3 @@
+       ret <1 x i128> %result
+; CHECK vadduqm 2, 2, 3*
+}
----------------
nemanjai wrote:
> 1. Why the star at the end?
> 2. I think that in a large test case, it helps to have CHECK-LABEL for every function so that if something fails, it is easier to find where the failure is. The tool won't go past a check if the previous one did not match.
The star is a typo.
I've also added the check label for the first several functions. Good call. 

================
Comment at: test/CodeGen/PowerPC/vec_add_sub_quadword.ll:71
@@ +70,3 @@
+; CHECK-LABEL: test_vaddeuqm
+; CHECK: vaddeuqm 2, 2, 3, 4
+}
----------------
nemanjai wrote:
> Would it make sense to check that the right parameter is in the right register?
I don't know how this would be done, short of actually running the test case and making sure the output is correct.

http://reviews.llvm.org/D9081

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list