[PATCH] D26546: [PPC] Add vec_insert4b/vec_extract4b to altivec.h

[Nemanja Ivanovic via cfe-commits]

nemanjai added inline comments.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:8182
+    ConstantInt *ArgCI = dyn_cast<ConstantInt>(Ops[2]);
+    assert(ArgCI);
+    int64_t index = clamp(ArgCI->getSExtValue(), 0, 12);
----------------
```assert(ArgCI && "The third operand of this intrinsic must be a compile-time constant")```


================
Comment at: lib/CodeGen/CGBuiltin.cpp:8183
+    assert(ArgCI);
+    int64_t index = clamp(ArgCI->getSExtValue(), 0, 12);
+
----------------
s/index/Index


================
Comment at: lib/CodeGen/CGBuiltin.cpp:8191
+      // Create a shuffle mask of (1, 0)
+      Constant *ShuffleElts[2];
+      ShuffleElts[0] = ConstantInt::get(Int32Ty, 1);
----------------
Just a minor nit, but I'd initialize this inline here.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:8213
+    ConstantInt *ArgCI = dyn_cast<ConstantInt>(Ops[1]);
+    assert(ArgCI);
+    int64_t index = clamp(ArgCI->getSExtValue(), 0, 12);
----------------
Same comment as above - make sure your asserts tell the developer why they trip.


================
Comment at: test/CodeGen/builtins-ppc-p9vector.c:1183
 }
+vector unsigned char test116(void) {
+// CHECK-BE: [[T1:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> {{.+}}, <2 x i64> {{.+}}, i32 7)
----------------
Add testing with an argument out of range as well as non-constant (presumably in a separate test case for the latter).


Repository:
  rL LLVM

https://reviews.llvm.org/D26546