[cfe-commits] Patch for pr9027

[jahanian]

On Jan 25, 2013, at 2:30 PM, John McCall <rjmccall at apple.com> wrote:

> On Jan 25, 2013, at 12:28 PM, jahanian <fjahanian at apple.com> wrote:
>> Please review this patch for pr9027 (also // rdar://11861085)
>> 
>> Title: [PR9027] volatile struct bug: member is not loaded at -O;
>> 
>> This is caused by last flag passed to @llvm.memcpy being false, not honoring that
>> aggregate has at least one 'volatile' data member (even though aggregate itself has
>> not been qualified as 'volatile'. As a result, optimization optimizes away the memcpy altogether.
>> 
>> Fix is combination of Sema and IRGen changes.
> 
>   HasObjectMember = false;
> +    HasVolatileMember = false;
> 
> Weird indentation.
> 
> +    if (const RecordType *RT = T->getAs<RecordType>())
> +      if (const RecordDecl *RD = cast<RecordDecl>(RT->getDecl()))
> 
> This inner condition is always true.
> 
> +  /// IsAggrVolatile - returns true if aggregate type has a volatile
> +  /// member.
> +  bool IsAggrVolatile(QualType T) {
> +    if (const RecordType *RT = T->getAs<RecordType>())
> +      if (const RecordDecl *RD = cast<RecordDecl>(RT->getDecl()))
> +        return RD->hasVolatileMember();
> +    return false;
> +  }
> 
> Please rename this to "hasVolatileMember".
> 
> +// CHECK: call void @llvm.memcpy
> +// CHECK: call void @llvm.memcpy
> +// CHECK: call void @llvm.memcpy
> +
> 
> Tests like this can be frustrating to analyze, and it's not too hard for them to
> succeed for spurious reasons.  Please test the IR with a little more
> specificity — in particular, you should test that we pass the volatile flag
> on the copies, that the copies occur in the right function, etc.
> 
In r173535 and r173543
- Fariborz

> John.