[cfe-commits] r173535 - patch for PR9027 and // rdar://11861085
Fariborz Jahanian
fjahanian at apple.com
Fri Jan 25 15:57:06 PST 2013
Author: fjahanian
Date: Fri Jan 25 17:57:05 2013
New Revision: 173535
URL: http://llvm.org/viewvc/llvm-project?rev=173535&view=rev
Log:
patch for PR9027 and // 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.
Patch review by John MaCall (I still need to fix up a test though).
Added:
cfe/trunk/test/CodeGen/no-opt-volatile-memcpy.c
cfe/trunk/test/CodeGenCXX/no-opt-volatile-memcpy.cpp
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGValue.h
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Fri Jan 25 17:57:05 2013
@@ -2930,6 +2930,10 @@ class RecordDecl : public TagDecl {
/// HasObjectMember - This is true if this struct has at least one member
/// containing an Objective-C object pointer type.
bool HasObjectMember : 1;
+
+ /// HasVolatileMember - This is true if struct has at least one member of
+ /// 'volatile' type.
+ bool HasVolatileMember : 1;
/// \brief Whether the field declarations of this record have been loaded
/// from external storage. To avoid unnecessary deserialization of
@@ -2986,6 +2990,9 @@ public:
bool hasObjectMember() const { return HasObjectMember; }
void setHasObjectMember (bool val) { HasObjectMember = val; }
+ bool hasVolatileMember() const { return HasVolatileMember; }
+ void setHasVolatileMember (bool val) { HasVolatileMember = val; }
+
/// \brief Determines whether this declaration represents the
/// injected class name.
///
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri Jan 25 17:57:05 2013
@@ -2734,6 +2734,7 @@ RecordDecl::RecordDecl(Kind DK, TagKind
HasFlexibleArrayMember = false;
AnonymousStructOrUnion = false;
HasObjectMember = false;
+ HasVolatileMember = false;
LoadedFieldsFromExternalStorage = false;
assert(classof(static_cast<Decl*>(this)) && "Invalid Kind!");
}
Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Jan 25 17:57:05 2013
@@ -301,6 +301,9 @@ CXXRecordDecl::setBases(CXXBaseSpecifier
// has an Objective-C object member.
if (BaseClassDecl->hasObjectMember())
setHasObjectMember(true);
+
+ if (BaseClassDecl->hasVolatileMember())
+ setHasVolatileMember(true);
// Keep track of the presence of mutable fields.
if (BaseClassDecl->hasMutableFields())
@@ -751,6 +754,8 @@ void CXXRecordDecl::addedMember(Decl *D)
data().HasIrrelevantDestructor = false;
if (FieldRec->hasObjectMember())
setHasObjectMember(true);
+ if (FieldRec->hasVolatileMember())
+ setHasVolatileMember(true);
// C++0x [class]p7:
// A standard-layout class is a class that:
Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Fri Jan 25 17:57:05 2013
@@ -792,6 +792,11 @@ void AggExprEmitter::VisitBinAssign(cons
AggValueSlot::forLValue(LHS, AggValueSlot::IsDestructed,
needsGC(E->getLHS()->getType()),
AggValueSlot::IsAliased);
+ // A non-volatile aggregate destination might have volatile member.
+ if (!LHSSlot.isVolatile() &&
+ CGF.hasVolatileMember(E->getLHS()->getType()))
+ LHSSlot.setVolatile(true);
+
CGF.EmitAggExpr(E->getRHS(), LHSSlot);
// Copy into the destination if the assignment isn't ignored.
Modified: cfe/trunk/lib/CodeGen/CGValue.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGValue.h?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGValue.h (original)
+++ cfe/trunk/lib/CodeGen/CGValue.h Fri Jan 25 17:57:05 2013
@@ -412,6 +412,10 @@ public:
return Quals.hasVolatile();
}
+ void setVolatile(bool flag) {
+ Quals.setVolatile(flag);
+ }
+
Qualifiers::ObjCLifetime getObjCLifetime() const {
return Quals.getObjCLifetime();
}
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Jan 25 17:57:05 2013
@@ -1665,14 +1665,24 @@ public:
void EmitExprAsInit(const Expr *init, const ValueDecl *D,
LValue lvalue, bool capturedByInit);
+ /// hasVolatileMember - returns true if aggregate type has a volatile
+ /// member.
+ bool hasVolatileMember(QualType T) {
+ if (const RecordType *RT = T->getAs<RecordType>()) {
+ const RecordDecl *RD = cast<RecordDecl>(RT->getDecl());
+ return RD->hasVolatileMember();
+ }
+ return false;
+ }
/// EmitAggregateCopy - Emit an aggrate assignment.
///
/// The difference to EmitAggregateCopy is that tail padding is not copied.
/// This is required for correctness when assigning non-POD structures in C++.
void EmitAggregateAssign(llvm::Value *DestPtr, llvm::Value *SrcPtr,
- QualType EltTy, bool isVolatile=false,
- CharUnits Alignment = CharUnits::Zero()) {
- EmitAggregateCopy(DestPtr, SrcPtr, EltTy, isVolatile, Alignment, true);
+ QualType EltTy) {
+ bool IsVolatile = hasVolatileMember(EltTy);
+ EmitAggregateCopy(DestPtr, SrcPtr, EltTy, IsVolatile, CharUnits::Zero(),
+ true);
}
/// EmitAggregateCopy - Emit an aggrate copy.
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jan 25 17:57:05 2013
@@ -10484,6 +10484,8 @@ void Sema::ActOnFields(Scope* S,
}
if (Record && FDTTy->getDecl()->hasObjectMember())
Record->setHasObjectMember(true);
+ if (Record && FDTTy->getDecl()->hasVolatileMember())
+ Record->setHasVolatileMember(true);
} else if (FDTy->isObjCObjectType()) {
/// A field cannot be an Objective-c object
Diag(FD->getLocation(), diag::err_statically_allocated_object)
@@ -10530,6 +10532,8 @@ void Sema::ActOnFields(Scope* S,
}
}
}
+ if (Record && FD->getType().isVolatileQualified())
+ Record->setHasVolatileMember(true);
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jan 25 17:57:05 2013
@@ -475,6 +475,7 @@ void ASTDeclReader::VisitRecordDecl(Reco
RD->setHasFlexibleArrayMember(Record[Idx++]);
RD->setAnonymousStructOrUnion(Record[Idx++]);
RD->setHasObjectMember(Record[Idx++]);
+ RD->setHasVolatileMember(Record[Idx++]);
}
void ASTDeclReader::VisitValueDecl(ValueDecl *VD) {
Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=173535&r1=173534&r2=173535&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Fri Jan 25 17:57:05 2013
@@ -263,6 +263,7 @@ void ASTDeclWriter::VisitRecordDecl(Reco
Record.push_back(D->hasFlexibleArrayMember());
Record.push_back(D->isAnonymousStructOrUnion());
Record.push_back(D->hasObjectMember());
+ Record.push_back(D->hasVolatileMember());
if (!D->hasAttrs() &&
!D->isImplicit() &&
@@ -1456,6 +1457,7 @@ void ASTWriter::WriteDeclsBlockAbbrevs()
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // FlexibleArrayMember
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // AnonymousStructUnion
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // hasObjectMember
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // hasVolatileMember
// DC
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // LexicalOffset
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // VisibleOffset
Added: cfe/trunk/test/CodeGen/no-opt-volatile-memcpy.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/no-opt-volatile-memcpy.c?rev=173535&view=auto
==============================================================================
--- cfe/trunk/test/CodeGen/no-opt-volatile-memcpy.c (added)
+++ cfe/trunk/test/CodeGen/no-opt-volatile-memcpy.c Fri Jan 25 17:57:05 2013
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -O0 -triple=x86_64-apple-darwin -emit-llvm -o - %s | FileCheck %s
+// rdar://11861085
+
+struct s {
+ char filler [128];
+ volatile int x;
+};
+
+struct s gs;
+
+void foo (void) {
+ struct s ls;
+ ls = ls;
+ gs = gs;
+ ls = gs;
+}
+// CHECK: define void @foo()
+// CHECK: %[[LS:.*]] = alloca %struct.s, align 4
+// CHECK-NEXT: %[[ZERO:.*]] = bitcast %struct.s* %[[LS]] to i8*
+// CHECK-NEXT: %[[ONE:.*]] = bitcast %struct.s* %[[LS]] to i8*
+// CHECK-NEXT: call void @llvm.memcpy.{{.*}}(i8* %[[ZERO]], i8* %[[ONE]], i64 132, i32 4, i1 true)
+// CHECK-NEXT: call void @llvm.memcpy.{{.*}}(i8* getelementptr inbounds (%struct.s* @gs, i32 0, i32 0, i32 0), i8* getelementptr inbounds (%struct.s* @gs, i32 0, i32 0, i32 0), i64 132, i32 4, i1 true)
+// CHECK-NEXT: %[[TWO:.*]] = bitcast %struct.s* %[[LS]] to i8*
+// CHECK-NEXT: call void @llvm.memcpy.{{.*}}(i8* %[[TWO]], i8* getelementptr inbounds (%struct.s* @gs, i32 0, i32 0, i32 0), i64 132, i32 4, i1 true)
+
+
+struct s1 {
+ struct s y;
+};
+
+struct s1 s;
+
+void fee (void) {
+ s = s;
+ s.y = gs;
+}
+// CHECK: define void @fee()
+// CHECK: call void @llvm.memcpy.{{.*}}(i8* getelementptr inbounds (%struct.s1* @s, i32 0, i32 0, i32 0, i32 0), i8* getelementptr inbounds (%struct.s1* @s, i32 0, i32 0, i32 0, i32 0), i64 132, i32 4, i1 true)
+// CHECK-NEXT: call void @llvm.memcpy.{{.*}}(i8* getelementptr inbounds (%struct.s1* @s, i32 0, i32 0, i32 0, i32 0), i8* getelementptr inbounds (%struct.s* @gs, i32 0, i32 0, i32 0), i64 132, i32 4, i1 true)
+
Added: cfe/trunk/test/CodeGenCXX/no-opt-volatile-memcpy.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/no-opt-volatile-memcpy.cpp?rev=173535&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/no-opt-volatile-memcpy.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/no-opt-volatile-memcpy.cpp Fri Jan 25 17:57:05 2013
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -O -triple=x86_64-apple-darwin -emit-llvm -o - %s | FileCheck %s
+// rdar://11861085
+
+struct s {
+ char filler [128];
+ volatile int x;
+};
+
+struct s gs;
+
+void foo (void) {
+ struct s ls;
+ ls = ls;
+ gs = gs;
+ ls = gs;
+}
+// CHECK: call void @llvm.memcpy
+// CHECK: call void @llvm.memcpy
+// CHECK: call void @llvm.memcpy
+
+struct s1 {
+ struct s y;
+};
+
+struct s1 s;
+
+void fee (void) {
+ s = s;
+ s.y = gs;
+}
+// CHECK: call void @llvm.memcpy
+// CHECK: call void @llvm.memcpy
+
+
+struct d : s1 {
+};
+
+d gd;
+
+void gorf(void) {
+ gd = gd;
+}
+// CHECK: call void @llvm.memcpy
+
More information about the cfe-commits
mailing list