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 the same type of vectors (any legal vector type) for the first
+// two arguments and takes compile time constant (0-3) for the third argument.
+// If a constant larger than 3 is used, only the last two bits of it are used.
----------------
This statement doesn't belong in this comment. The Sema check is just that the
third argument is a compile-time constant. There's no need to mention `0-3` or
`last two bits` here. Just state what you're checking, not the details of what
you're currently using it for.
================
Comment at: lib/Sema/SemaChecking.cpp:3905
+// vector short vec_xxsldwi(vector short, vector short, int);
+bool Sema::SemaBuiltinVSX(CallExpr *TheCall, unsigned NumArgs) {
+ if (TheCall->getNumArgs() < NumArgs)
----------------
All the statements in the comments as well as the code itself only handle
functions that have exactly 3 parameters, the first two of which are vectors
and the last an integral constant expression. I really don't see the need for
the `NumArgs` parameter here. It would indeed be weird if someone down the line
called it with something like:
`SemaBuiltinVSX(TheCall, 1);`
or
`SemaBuiltinVSX(TheCall, 5);`
https://reviews.llvm.org/D33053
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
