<div dir="ltr">I've started looking at cleaning this up a bit more.<div><br></div><div>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?<div>
<br></div><div><br></div><div>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</div><div><br></div><div>"first two arguments to __builtin_shufflevector must have the same type"<br>
</div><div><br></div><div>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?</div>
<div><br></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 19, 2013 at 11:01 AM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Thu, Jul 18, 2013 at 10:03 PM, Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>> wrote:<br>

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