echristo closed this revision.
echristo added a comment.
This was committed:
commit d65cd1f9424369c4ae7f945fac7fd9e4357451b2
Author: Sean Fertile
Date: Thu Jan 5 21:43:30 2017 +
Add vec_insert4b and vec_extract4b functions to altivec.h
Add builtins for the functions and custom code
nemanjai added inline comments.
Comment at: test/CodeGen/builtins-ppc-extractword-error.c:2
+// REQUIRES: powerpc-registered-target
+// XFAIL: powerpc
+
I think this will fail on all the powerpc targets, such as powerpc64le, etc.
Which isn't what you want I imag
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.
Just one minor inline nit. Other than that, LGTM.
Comment at: test/CodeGen/builtins-ppc-error.c:5
+// RUN: -triple powerpc64-unknown-unknown -fsyntax-only \
+// RUN:
sfertile updated this revision to Diff 82031.
sfertile marked 6 inline comments as done.
sfertile added a comment.
Updated to swap the arguments when generating the intrinsic. Updated a number
of the comments, and added some tests with the index out of range.
Repository:
rL LLVM
https://revi
nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
The finalized wording for vec_insert4b:
Let W be the first doubleword element of ARG1, truncated to 32 bits. The
result vector is formed by inserting W into ARG2 at the byte posi
nemanjai added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8193
+if (getTarget().isLittleEndian()) {
+ // Create a shuffle mask of (1, 0)
+ Constant *ShuffleElts[2] = { ConstantInt::get(Int32Ty, 1),
This will likely have to change when th
sfertile updated this revision to Diff 79860.
sfertile marked an inline comment as done.
sfertile added a comment.
Changed all variable names to start with capitals, added description strings to
the assertions, changed uses of '12' to MaxIndex and initialized the arrays in
their definition.
Re
nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
Overall, I'd like to see another revision of this. This is not because I feel
the patch requires major rework, but because there is a number of minor
comments that I'd like to ma
kbarton accepted this revision.
kbarton added a comment.
This revision is now accepted and ready to land.
LGTM, but I'd like @nemanjai to have a quick look at the
__builtin_vsx_insertword and __builtin_vsx_extractuword too.
Repository:
rL LLVM
https://reviews.llvm.org/D26546
_
nemanjai added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8182
+ConstantInt *ArgCI = dyn_cast(Ops[2]);
+assert(ArgCI);
+int64_t index = clamp(ArgCI->getSExtValue(), 0, 12);
```assert(ArgCI && "The third operand of this intrinsic must be a
sfertile marked an inline comment as done.
sfertile added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8185
+
+// Need to cast the second argument from a vector of cahr to a vector
+// of long long.
syzaara wrote:
> tiny comment, char is misspel
sfertile updated this revision to Diff 78911.
sfertile added a comment.
Fixed spelling error in comment
Repository:
rL LLVM
https://reviews.llvm.org/D26546
Files:
include/clang/Basic/BuiltinsPPC.def
lib/CodeGen/CGBuiltin.cpp
lib/Headers/altivec.h
test/CodeGen/builtins-ppc-p9vector.c
syzaara added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8185
+
+// Need to cast the second argument from a vector of cahr to a vector
+// of long long.
tiny comment, char is misspelled as cahr
Repository:
rL LLVM
https://reviews.llvm.org
sfertile updated this revision to Diff 78760.
sfertile added a comment.
Moved the endian related massaging from altivec.h into Clang codegen and
clamped the input index into the valid range [0, 12].
Repository:
rL LLVM
https://reviews.llvm.org/D26546
Files:
include/clang/Basic/BuiltinsPPC
kbarton added a comment.
Did you upload a new patch? The diff doesn't appear to change for me.
Repository:
rL LLVM
https://reviews.llvm.org/D26546
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
sfertile added inline comments.
Comment at: lib/Headers/altivec.h:11908
+#define vec_extract4b(__a, __b)
\
+ vec_reve((vector unsigned long long)
\
+__builtin_vsx_xxextractuw
nemanjai added inline comments.
Comment at: lib/Headers/altivec.h:11908
+#define vec_extract4b(__a, __b)
\
+ vec_reve((vector unsigned long long)
\
+__builtin_vsx_xxextractuw
kbarton added inline comments.
Comment at: lib/Headers/altivec.h:12014
+#define vec_insert4b(__a, __b, __c) \
+ ((vector unsigned char)__builtin_vsx_xxinsertw((__a), (__b), (__c) & 0xF))
+#endif
nemanjai wrote:
> As far as I can tell by looking at this patch and
nemanjai added inline comments.
Comment at: lib/Headers/altivec.h:12014
+#define vec_insert4b(__a, __b, __c) \
+ ((vector unsigned char)__builtin_vsx_xxinsertw((__a), (__b), (__c) & 0xF))
+#endif
As far as I can tell by looking at this patch and the correspondin
sfertile created this revision.
sfertile added reviewers: amehsan, kbarton, nemanjai, jtony, syzaara, lei.
sfertile added subscribers: cfe-commits, echristo.
sfertile set the repository for this revision to rL LLVM.
Add macros that implement the vec_extract4b and vec_insert4b functionality.
vecto
20 matches
Mail list logo
