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

Nemanja Ivanovic nemanja.i.ibm at gmail.com
Fri Apr 17 13:34:09 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
----------------
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.

================
Comment at: test/CodeGen/PowerPC/vec_add_sub_quadword.ll:9
@@ +8,3 @@
+       ret <1 x i128> %result
+; CHECK: vadduqm 2, 2, 3
+}
----------------
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.

================
Comment at: test/CodeGen/PowerPC/vec_add_sub_quadword.ll:15
@@ +14,3 @@
+       ret <1 x i128> %result
+; CHECK vadduqm 2, 2, 3*
+}
----------------
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.

================
Comment at: test/CodeGen/PowerPC/vec_add_sub_quadword.ll:71
@@ +70,3 @@
+; CHECK-LABEL: test_vaddeuqm
+; CHECK: vaddeuqm 2, 2, 3, 4
+}
----------------
Would it make sense to check that the right parameter is in the right register?

http://reviews.llvm.org/D9081

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






More information about the llvm-commits mailing list