[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