[cfe-commits] Patch for pr9027
jahanian
fjahanian at apple.com
Fri Jan 25 17:30:46 PST 2013
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.
More information about the cfe-commits
mailing list