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

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

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

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

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

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:

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

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

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

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

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

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

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

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.

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

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

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

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

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

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

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

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

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

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

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

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

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

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:

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

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

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

sfertile added inline comments. Comment at: lib/Headers/altivec.h:11908 +#define vec_extract4b(__a, __b) \ + vec_reve((vector unsigned long long) \ +

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

nemanjai added inline comments. Comment at: lib/Headers/altivec.h:11908 +#define vec_extract4b(__a, __b) \ + vec_reve((vector unsigned long long) \ +

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

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

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

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