[PATCH] D82520: [Power10] Implement Vector Splat Immediate Builtins in LLVM/Clang

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 2 03:10:31 PDT 2020


nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.

If this patch does not include the back end codegen test cases for these, which patch does? I don't see any selection patterns or test cases for `xxsplti32dx` in ToT.
The implementation and test cases for `xxspltiw` are in ToT, so they don't appear to be a concern.



================
Comment at: clang/lib/Headers/altivec.h:16903
+
+#ifdef __LITTLE_ENDIAN__
+static __inline__ vector signed int __ATTRS_o_ai vec_splati_ins(
----------------
Again, I think it is preferable from a readability standpoint to keep the LE and BE blocks close together:
```
static __inline__ vector signed int __ATTRS_o_ai vec_splati_ins(
    vector signed int __a, const unsigned int __b, const signed int __c) {
  assert(__b < 2 && "The second argument must be 0 or 1");
#ifdef __LITTLE_ENDIAN__
  __a[1 - __b] = __c;
  __a[3 - __b] = __c;
#else
  __a[__b] = __c;
  __a[2 + __b] = __c;
#endif
  return __a;
}
```
And similarly for the other two. Of course, in this case you can't really forward calls because there isn't really a way to represent a bitcast of a float constant to int in the language.


================
Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:184
+vector signed int test_vec_vec_splati_ins_si(void) {
+  // CHECK-BE: insertelement <4 x i32>
+  // CHECK-BE: ret <4 x i32>
----------------
You have separate checks for LE/BE. Please show the index in the checks and don't use the same index for all 3 test cases.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82520/new/

https://reviews.llvm.org/D82520





More information about the llvm-commits mailing list