[cfe-dev] __builtin_shufflevector question

[Craig Topper]

I've started looking at cleaning this up a bit more.

I'm leaning towards removing form 2 support from CGExprScalar.cpp since it
doesn't work in Sema and therefore could never have been used/tested. I'm
not even 100% sure what the semantics are supposed to be as far as widths
of the individual arguments relative to each other. Thoughts?


Additionally, he error messages in SemaChecking here are a bit misleading
given the overloaded nature of this instruction. For instance, we have a
message that says

"first two arguments to __builtin_shufflevector must have the same type"

This error gets used for form 3 and when form 1 has a mask of non-integer
integer type or a mismatch in the number of elements. Clearly form 1 needs
clearer error messages for those cases. But then you end up needing to
specify which form of __builtin_shufflevector in all of the error messages
to avoid statements like "first two arguments must be..." which is only
true for one of the forms. Or should we just have separate __builtin names
for the constant index form and the mask form?



On Fri, Jul 19, 2013 at 11:01 AM, Eli Friedman <eli.friedman at gmail.com>wrote:

> On Thu, Jul 18, 2013 at 10:03 PM, Craig Topper <craig.topper at gmail.com>
> wrote:
> > And another question, should -1 be a valid index to specify undef?
>
> I can't think of a better way to specify undef in the general case.
>
> > On Thu, Jul 18, 2013 at 9:58 PM, Craig Topper <craig.topper at gmail.com>
> > wrote:
> >>
> >> What are the supported forms of __builtin_shufflevector supposed to be?
> >>
> >> Comments in SemaChecking.cpp indicates these
> >>   // 1) unary, vector mask: (lhs, mask)
> >>   // 2) binary, vector mask: (lhs, rhs, mask)
> >>   // 3) binary, scalar mask: (lhs, rhs, index, ..., index)
> >>
> >> But the actual code in SemaChecking.cpp only implements 1 and 3. I tried
> >> to use 2 and it fails with "index for __builtin_shufflevector must be a
> >> constant integer" on the 3rd argument.
> >>
> >>
> >> Additionally, official documentation indicates only form 3 is supported.
> >> http://clang.llvm.org/docs/LanguageExtensions.html
>
> IIRC, the original __builtin_shufflevector only supported form 3;
> forms 1 and 2 got added in a later patch, and I guess the patch wasn't
> very good quality.
>
> -Eli
>



-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130728/92bb8eb3/attachment.html>