This revision was automatically updated to reflect the committed changes.
jtony marked 2 inline comments as done.
Closed by commit rL303760: [PowerPC] Implement vec_xxpermdi builtin. (authored
by jtony).
Changed prior to commit:
https://reviews.llvm.org/D33053?vs=99966&id=100093#toc
Repository
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.
Much like Eric's comments, mine shouldn't hold up approval. Feel free to
address them on the commit.
LGTM.
Comment at: lib/Sema/SemaChecking.cpp:3900
+// Which takes th
jtony updated this revision to Diff 99966.
https://reviews.llvm.org/D33053
Files:
include/clang/Basic/BuiltinsPPC.def
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/CodeGen/CGBuiltin.cpp
lib/Headers/altivec.h
lib/Sema/SemaChecking.cpp
test/CodeGen/builtins-
echristo accepted this revision.
echristo added a comment.
Couple of small nits and a request to make some of the change separately, but
otherwise LGTM. For the split part please don't actually submit another patch,
just go ahead and do it :)
Thanks!
-eric
Comment at: inclu
inouehrs added inline comments.
Comment at: test/CodeGen/builtins-ppc-error.c:23
+void testXXPERMDI(void) {
+ int index = 5;
+ vec_xxpermdi(vsi); //expected-error {{too few arguments to function call,
expected at least 3, have 1}}
jtony wrote:
> inouehrs wrote
jtony updated this revision to Diff 99955.
jtony marked 6 inline comments as done.
jtony added a comment.
Address more comments from Nemanja and Hiroshi.
https://reviews.llvm.org/D33053
Files:
include/clang/Basic/BuiltinsPPC.def
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sem
jtony added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:3900
+// vector short vec_xxsldwi(vector short, vector short, int);
+bool Sema::SemaBuiltinVSX(CallExpr *TheCall, unsigned NumArgs) {
+ if (TheCall->getNumArgs() < NumArgs)
nemanjai wrote:
> I as
inouehrs added inline comments.
Comment at: test/CodeGen/builtins-ppc-error.c:23
+void testXXPERMDI(void) {
+ int index = 5;
+ vec_xxpermdi(vsi); //expected-error {{too few arguments to function call,
expected at least 3, have 1}}
I am not sure we can assure t
nemanjai added a comment.
Other than the few minor comments, this LGTM.
Comment at: lib/CodeGen/CGBuiltin.cpp:8458
+if (getTarget().isLittleEndian()) {
+ ElemIdx0 = (~Index & 1) + 2;
+ ElemIdx1 = (~Index & 2) >> 1;
Minor nit: please add a comment
jtony updated this revision to Diff 99292.
jtony added a comment.
Address all the comments from Nemanja.
https://reviews.llvm.org/D33053
Files:
include/clang/Basic/BuiltinsPPC.def
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/CodeGen/CGBuiltin.cpp
lib/Header
jtony marked 6 inline comments as done.
jtony added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8433
+if (getTarget().isLittleEndian()) {
+ switch (Index) {
+ case 0:
nemanjai wrote:
> The switch is overkill. You should just implement thi
nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
Add a test case like the one that currently crashes (see inline comment). Also,
please do the following:
- Put a note in the description (and the commit message) with a link to t
jtony created this revision.
The vec_xxpermdi builtin is missing from altivec.h. This has been requested by
developers working on libvpx for VP9 support for Google. Initially, I tried to
define a new intrinsic to map it to the corresponding PowerPC hard instruction
(XXPERMDI) directly. But the
13 matches
Mail list logo
