[cfe-commits] r77332 - in /cfe/trunk: lib/CodeGen/CGCXX.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.h test/CodeGenCXX/constructor-init.cpp

[Fariborz Jahanian]

On Jul 28, 2009, at 3:15 PM, Chris Lattner wrote:

> On Jul 28, 2009, at 10:38 AM, Fariborz Jahanian wrote:
>> Author: fjahanian
>> Date: Tue Jul 28 12:38:28 2009
>> New Revision: 77332
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=77332&view=rev
>> Log:
>> More work toward data member access ir-gen.
>
> Nice.  :)
>
>> +llvm::Value *CodeGenFunction::AddressCXXOfBaseClass(llvm::Value  
>> *BaseValue,
>> +                                                CXXRecordDecl  
>> *ClassDecl,
>> +                                                CXXRecordDecl  
>> *BaseClassDecl) {
>> +  if (ClassDecl == BaseClassDecl)
>> +    return BaseValue;
>> +
>> +  // Accessing a member of the base class. Must add delata to
>> +  // the load of 'this'.
>> +  // FIXME. Once type layout is complete, this will probably change.
>> +  const ASTRecordLayout &Layout =
>> +  getContext().getASTRecordLayout(ClassDecl);
>> +  llvm::Type *I8Ptr =  
>> VMContext.getPointerTypeUnqual(llvm::Type::Int8Ty);
>> +  unsigned Idx = 0;
>> +  for (CXXRecordDecl::base_class_const_iterator i =
>> +         ClassDecl->bases_begin(),
>> +         e = ClassDecl->bases_end(); i != e; ++i, ++Idx) {
>> +    if (!i->isVirtual()) {
>> +        const CXXRecordDecl *Base =
>> +        cast<CXXRecordDecl>(i->getType()->getAsRecordType()- 
>> >getDecl());
>> +        if (Base == BaseClassDecl)
>> +          break;
>> +    }
>> +  }
>
> Please split this out into a static function that return Idx, as per  
> the previous email.
>
>> +  uint64_t Offset = Layout.getFieldOffset(Idx) / 8;
>
> Maybe there should be a getFieldOffsetInBytes() method?

This code will all likelihood change when Anders has the type layout
change done.
Base class offset calculation is based on the current layout and API   
and will
be replaced soon.

>
>
>> +  llvm::Value *OffsetVal =  
>> llvm::ConstantInt::get(llvm::Type::Int32Ty, Offset);
>
> will this work with large (64-bit) offsets on x86-64?  Maybe this  
> should use pointertype instead?
>
>>
>>    CVRQualifiers = PTy->getPointeeType().getCVRQualifiers();
>> +    if (CXXThisExpr *ThisExpr = dyn_cast<CXXThisExpr>(BaseExpr)) {
>> +      QualType ClassTy = ThisExpr->getType();
>> +      ClassTy = ClassTy->getPointeeType();
>> +      CXXRecordDecl *ClassDecl =
>> +        cast<CXXRecordDecl>(ClassTy->getAsRecordType()->getDecl());
>
> I'm seeing a lot of this sort of code.  Can you add a method on  
> Type, something like "getCXXRecordDeclForPointerType()" that returns  
> a CXXRecordDecl if the type is a pointer to record type?
>
> It would basically be:
>
>  if (PointerType *PT = X->getAsPointerType())
>    if (RecordType *RT = PT->getPointeeType()->getAsRecordType())
>      return dyn_cast<CXXRecordDecl>(RT);
>  return 0;
>

Good idea. Will do so shortly.

- Fariborz

> Please make use of this helper in various other places, it seems  
> that it could significantly simplify a lot of the code you've been  
> working on lately.
>
> -Chris
>