[cfe-dev] OpenCL patch: vector literals (alternative patch)

[Tanya Lattner]

Attaching patch that actually has the test cases that Anton wrote:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenCL-VectorLiterals1.patch
Type: application/octet-stream
Size: 4718 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110713/7bd1630b/attachment.obj>
-------------- next part --------------


-Tanya

On Jul 13, 2011, at 11:11 AM, Chris Lattner wrote:

> Seems reasonable to me, but needs a testcase.
> 
> -Chris
> 
> On Jul 13, 2011, at 10:44 AM, Tanya Lattner wrote:
> 
>> Any other comments on this patch? I'd like to commit.
>> 
>> -Tanya
>> 
>> On Jul 7, 2011, at 4:41 PM, Tanya Lattner wrote:
>> 
>>> I'm attaching an alternative patch which handles the missing vector splat case. It passes all the valid and invalid test cases you provided and I included those tests in the patch. It also uses the fix in SemaInit that you included which handles the crash for one invalid case.
>>> 
>>> Thanks,
>>> Tanya
>>> 
>>> <OpenCL-VectorLiterals.patch>
>>> On Jul 7, 2011, at 8:17 AM, Anton Lokhmotov wrote:
>>> 
>>>> Hi Tanya,
>>>> 
>>>> Many thanks for your prompt review!
>>>> 
>>>>> All the support already exists for all the valid test cases 
>>>>> except the last one ((float4)( float )). One just has to modify 
>>>>> ActOnCastOfParenListExpr which is now BuildVectorLiteral to handle 
>>>>> the splat case. I have not modified our patch to match the recent 
>>>>> changes in TOT, but will do shortly.
>>>> 
>>>> We would be happy with minimal changes to Clang provided that the OpenCL
>>>> requirements are properly met, including handling invalid code.  We
>>>> specifically avoided using/modifying/breaking the existing functionality for
>>>> AltiVec because the OpenCL syntax is quite different. Thus, we believe that
>>>> for OpenCL CompilerInvocation::setLangDefaults() should disable Opts.AltiVec
>>>> (and OTOH enable Opts.Bool - but sorry about that slipping into this patch).
>>>> 
>>>>> Yes, you are 100% right that the test violates the OpenCL spec 
>>>>> (my mistake and haste in making a patch), but its catching a nasty
>>>>> compiler crash and I'd rather try to modify the test instead of just
>>>>> removing it. I've fixed this in TOT.
>>>> 
>>>> Unfortunately,
>>>> 
>>> 
>>>> void foo( uchar8 x )
>>>> {
>>>> uchar4 val[4] = {{(uchar4){x.lo}}};
>>>> }
>>>> 
>>>> is still invalid in OpenCL C.  In particular, OpenCL doesn't allow using
>>>> braces {} in vector literals.
>>>> 
>>>> [Actually, I'm quite confused by this test.  Since x.lo produces uchar4, the
>>>> cast is superfluous?  And then it attempts to initialize an array of vectors
>>>> val[] from vector x.lo.  But I actually would expect only val[0] to be
>>>> initialized (backed by my experiments with gcc).  But if it's just ensuring
>>>> that the compiler doesn't crash, that's fine (in the AltiVec mode).]
>>>> 
>>>>> For the invalid cases, I don't see why one needs to modify the parser, 
>>>>> but the last invalid case does crash the compiler which shouldn't ever
>>>>> happen.
>>>> 
>>>> Could you please ensure that the patch is applied correctly?  It works fine
>>>> with r134483.  Specifically, this modification prevents the compiler from
>>>> crashing:
>>>> 
>>>> [11:17:50] Vitaly Lugovskiy: diff --git a/lib/Sema/SemaInit.cpp
>>>> b/lib/Sema/SemaInit.cpp
>>>> index 16ba2a2..17af145 100644
>>>> --- a/lib/Sema/SemaInit.cpp
>>>> +++ b/lib/Sema/SemaInit.cpp
>>>> @@ -769,7 +769,8 @@ void InitListChecker::CheckSubElementType(const
>>>> InitializedEntity &Entity,
>>>> //   subaggregate, brace elision is assumed and the initializer is
>>>> //   considered for the initialization of the first member of
>>>> //   the subaggregate.
>>>> -  if (ElemType->isAggregateType() || ElemType->isVectorType()) {
>>>> +  if (!SemaRef.getLangOptions().OpenCL && 
>>>> +      (ElemType->isAggregateType() || ElemType->isVectorType())) {
>>>>  CheckImplicitInitList(Entity, IList, ElemType, Index, StructuredList,
>>>>                        StructuredIndex);
>>>>  ++StructuredIndex;
>>>> 
>>>> If you provide your modifications to support vector literals, we will run
>>>> our test cases with combinatorial coverage to ensure that the OpenCL
>>>> requirements are met.
>>>> 
>>>> Best wishes,
>>>> Anton.
>>>> 
>>>> 
>>>> 
>>> 
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> 
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>