[cfe-commits] Patch for pr9027

[John McCall]

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.

John.