[cfe-commits] r76717 - in /cfe/trunk: lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGValue.h lib/Sema/SemaExpr.cpp test/CodeGen/address-space-field1.c test/CodeGen/address-space-field2.c test/CodeGen/address-space-field3.c test/Co

[Mon Ping Wang]

Hi Eli,

I'll add some tests for Anonymous Structs and try to avoid using  
CodeGen to verify the type.  The embedded C spec treats address  
qualifiers the same way as volatile qualifier so I model the changes  
to support address spaces in these cases to be the same as volatile is  
treated. This means that I added it to the LValue.  Unlike volatile  
where we set the attribute on the instruction, the address qualifier  
is needed on the type, which is already encoded in Value* (as you  
correctly noted below) so it is currently not being used.

Thanks for the comments,
   -- Mon Ping



On Jul 21, 2009, at 9:21 PM, Eli Friedman wrote:

> On Tue, Jul 21, 2009 at 8:08 PM, Mon P Wang<wangmp at apple.com> wrote:
>> Author: wangmp
>> Date: Tue Jul 21 22:08:17 2009
>> New Revision: 76717
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=76717&view=rev
>> Log:
>> Preserve address space information through member accesses, e.g.,
>>   __attribute__((address_space(1))) struct {int arr[ 3 ]; }  *p1;
>>   ... = p1->arr[2];  // load from address space 1
>
> You're missing tests for the BuildAnonymousStructUnionMemberReference;
> otherwise, the Sema changes look okay.  But please try to add some
> tests that don't depend on CodeGen (a good way to verify types are
> correct is to declare an object by directly writing out the type, then
> re-declare it using an __typeof as the type).
>
> The CodeGen changes look wrong; the address-space should already be
> encoded in the type of the Value*, so it shouldn't be necessary to
> separately encode it explicitly.  Also, as far as I can tell (correct
> me if I'm wrong), they don't actually do anything, since you didn't
> add any uses of the AddressSpace member.
>
> -Eli