From a5986b9e91be95771a034b5efe400e99149f919c Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Sun, 3 Dec 2017 03:10:13 +0000 Subject: [PATCH] Revert "[CodeGen] Add initial support for union members in TBAA" This reverts commit r319413. See PR35503. We can't use "union member" as the access type here like this. llvm-svn: 319629 --- clang/lib/CodeGen/CGExpr.cpp | 31 ++++----- clang/lib/CodeGen/CodeGenModule.h | 5 +- clang/lib/CodeGen/CodeGenTBAA.cpp | 42 +++++-------- clang/lib/CodeGen/CodeGenTBAA.h | 19 +----- clang/test/CodeGen/tbaa-union.cpp | 100 ------------------------------ clang/test/CodeGen/union-tbaa1.c | 14 ++--- 6 files changed, 37 insertions(+), 174 deletions(-) delete mode 100644 clang/test/CodeGen/tbaa-union.cpp diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 50d116ec7d8c..88116f7d810e 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -3723,6 +3723,9 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, if (base.getTBAAInfo().isMayAlias() || rec->hasAttr() || FieldType->isVectorType()) { FieldTBAAInfo = TBAAAccessInfo::getMayAliasInfo(); + } else if (rec->isUnion()) { + // TODO: Support TBAA for unions. + FieldTBAAInfo = TBAAAccessInfo::getMayAliasInfo(); } else { // If no base type been assigned for the base access, then try to generate // one for this base lvalue. @@ -3733,26 +3736,16 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, "Nonzero offset for an access with no base type!"); } - // All union members are encoded to be of the same special type. - if (FieldTBAAInfo.BaseType && rec->isUnion()) - FieldTBAAInfo = TBAAAccessInfo::getUnionMemberInfo(FieldTBAAInfo.BaseType, - FieldTBAAInfo.Offset, - FieldTBAAInfo.Size); + // Adjust offset to be relative to the base type. + const ASTRecordLayout &Layout = + getContext().getASTRecordLayout(field->getParent()); + unsigned CharWidth = getContext().getCharWidth(); + if (FieldTBAAInfo.BaseType) + FieldTBAAInfo.Offset += + Layout.getFieldOffset(field->getFieldIndex()) / CharWidth; - // For now we describe accesses to direct and indirect union members as if - // they were at the offset of their outermost enclosing union. - if (!FieldTBAAInfo.isUnionMember()) { - // Adjust offset to be relative to the base type. - const ASTRecordLayout &Layout = - getContext().getASTRecordLayout(field->getParent()); - unsigned CharWidth = getContext().getCharWidth(); - if (FieldTBAAInfo.BaseType) - FieldTBAAInfo.Offset += - Layout.getFieldOffset(field->getFieldIndex()) / CharWidth; - - // Update the final access type. - FieldTBAAInfo.AccessType = CGM.getTBAATypeInfo(FieldType); - } + // Update the final access type. + FieldTBAAInfo.AccessType = CGM.getTBAATypeInfo(FieldType); } Address addr = base.getAddress(); diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index ff9866a36964..8b142933a7d1 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -688,9 +688,8 @@ public: /// getTBAAInfoForSubobject - Get TBAA information for an access with a given /// base lvalue. TBAAAccessInfo getTBAAInfoForSubobject(LValue Base, QualType AccessType) { - TBAAAccessInfo TBAAInfo = Base.getTBAAInfo(); - if (TBAAInfo.isMayAlias() || TBAAInfo.isUnionMember()) - return TBAAInfo; + if (Base.getTBAAInfo().isMayAlias()) + return TBAAAccessInfo::getMayAliasInfo(); return getTBAAAccessInfo(AccessType); } diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp index 80ad9aecfedd..2bc4b8a542bf 100644 --- a/clang/lib/CodeGen/CodeGenTBAA.cpp +++ b/clang/lib/CodeGen/CodeGenTBAA.cpp @@ -74,10 +74,6 @@ llvm::MDNode *CodeGenTBAA::getChar() { return Char; } -llvm::MDNode *CodeGenTBAA::getUnionMemberType(uint64_t Size) { - return createScalarTypeNode("union member", getChar(), Size); -} - static bool TypeHasMayAlias(QualType QTy) { // Tagged types have declarations, and therefore may have attributes. if (const TagType *TTy = dyn_cast(QTy)) @@ -105,8 +101,9 @@ static bool isValidBaseType(QualType QTy) { return false; if (RD->hasFlexibleArrayMember()) return false; - // For now, we do not allow interface classes to be base access types. - if (RD->isStruct() || RD->isClass() || RD->isUnion()) + // RD can be struct, union, class, interface or enum. + // For now, we only handle struct and class. + if (RD->isStruct() || RD->isClass()) return true; } return false; @@ -280,27 +277,18 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { const RecordDecl *RD = TTy->getDecl()->getDefinition(); const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); SmallVector Fields; - if (RD->isUnion()) { - // Unions are represented as structures with a single member that has a - // special type and occupies the whole object. - uint64_t Size = Context.getTypeSizeInChars(Ty).getQuantity(); - llvm::MDNode *TypeNode = getUnionMemberType(Size); - Fields.push_back(llvm::MDBuilder::TBAAStructField(/* Offset= */ 0, Size, - TypeNode)); - } else { - for (FieldDecl *Field : RD->fields()) { - QualType FieldQTy = Field->getType(); - llvm::MDNode *TypeNode = isValidBaseType(FieldQTy) ? - getBaseTypeInfo(FieldQTy) : getTypeInfo(FieldQTy); - if (!TypeNode) - return nullptr; + for (FieldDecl *Field : RD->fields()) { + QualType FieldQTy = Field->getType(); + llvm::MDNode *TypeNode = isValidBaseType(FieldQTy) ? + getBaseTypeInfo(FieldQTy) : getTypeInfo(FieldQTy); + if (!TypeNode) + return BaseTypeMetadataCache[Ty] = nullptr; - uint64_t BitOffset = Layout.getFieldOffset(Field->getFieldIndex()); - uint64_t Offset = Context.toCharUnitsFromBits(BitOffset).getQuantity(); - uint64_t Size = Context.getTypeSizeInChars(FieldQTy).getQuantity(); - Fields.push_back(llvm::MDBuilder::TBAAStructField(Offset, Size, - TypeNode)); - } + uint64_t BitOffset = Layout.getFieldOffset(Field->getFieldIndex()); + uint64_t Offset = Context.toCharUnitsFromBits(BitOffset).getQuantity(); + uint64_t Size = Context.getTypeSizeInChars(FieldQTy).getQuantity(); + Fields.push_back(llvm::MDBuilder::TBAAStructField(Offset, Size, + TypeNode)); } SmallString<256> OutName; @@ -345,8 +333,6 @@ llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) { if (Info.isMayAlias()) Info = TBAAAccessInfo(getChar(), Info.Size); - else if (Info.isUnionMember()) - Info.AccessType = getUnionMemberType(Info.Size); if (!Info.AccessType) return nullptr; diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h index 7b3473fe63fa..a5b1f66bcd1a 100644 --- a/clang/lib/CodeGen/CodeGenTBAA.h +++ b/clang/lib/CodeGen/CodeGenTBAA.h @@ -34,10 +34,9 @@ class CGRecordLayout; // TBAAAccessKind - A kind of TBAA memory access descriptor. enum class TBAAAccessKind : unsigned { - Ordinary, // An ordinary memory access. - MayAlias, // An access that may alias with any other accesses. - Incomplete, // Used to designate pointee values of incomplete types. - UnionMember, // An access to a direct or indirect union member. + Ordinary, + MayAlias, + Incomplete, }; // TBAAAccessInfo - Describes a memory access in terms of TBAA. @@ -78,14 +77,6 @@ struct TBAAAccessInfo { bool isIncomplete() const { return Kind == TBAAAccessKind::Incomplete; } - static TBAAAccessInfo getUnionMemberInfo(llvm::MDNode *BaseType, - uint64_t Offset, uint64_t Size) { - return TBAAAccessInfo(TBAAAccessKind::UnionMember, BaseType, - /* AccessType= */ nullptr, Offset, Size); - } - - bool isUnionMember() const { return Kind == TBAAAccessKind::UnionMember; } - bool operator==(const TBAAAccessInfo &Other) const { return Kind == Other.Kind && BaseType == Other.BaseType && @@ -157,10 +148,6 @@ class CodeGenTBAA { /// considered to be equivalent to it. llvm::MDNode *getChar(); - /// getUnionMemberType - Get metadata that represents the type of union - /// members. - llvm::MDNode *getUnionMemberType(uint64_t Size); - /// CollectFields - Collect information about the fields of a type for /// !tbaa.struct metadata formation. Return false for an unsupported type. bool CollectFields(uint64_t BaseOffset, diff --git a/clang/test/CodeGen/tbaa-union.cpp b/clang/test/CodeGen/tbaa-union.cpp deleted file mode 100644 index 066d7c9e9ff8..000000000000 --- a/clang/test/CodeGen/tbaa-union.cpp +++ /dev/null @@ -1,100 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s -emit-llvm -o - | FileCheck %s -// -// Check that we generate correct TBAA information for accesses to union -// members. - -struct X { - int a, b; - int arr[3]; - int c, d; -}; - -union U { - int i; - X x; - int j; -}; - -struct S { - U u, v; -}; - -union N { - int i; - S s; - int j; -}; - -struct R { - N n, m; -}; - -int f1(U *p) { -// CHECK-LABEL: _Z2f1P1U -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_U_j:!.*]] - return p->j; -} - -int f2(S *p) { -// CHECK-LABEL: _Z2f2P1S -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_u_i:!.*]] - return p->u.i; -} - -int f3(S *p) { -// CHECK-LABEL: _Z2f3P1S -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_v_j:!.*]] - return p->v.j; -} - -int f4(S *p) { -// CHECK-LABEL: _Z2f4P1S -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_u_x_b:!.*]] - return p->u.x.b; -} - -int f5(S *p) { -// CHECK-LABEL: _Z2f5P1S -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_v_x_b:!.*]] - return p->v.x.b; -} - -int f6(S *p) { -// CHECK-LABEL: _Z2f6P1S -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_u_x_arr:!.*]] - return p->u.x.arr[1]; -} - -int f7(S *p) { -// CHECK-LABEL: _Z2f7P1S -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_S_v_x_arr:!.*]] - return p->v.x.arr[1]; -} - -int f8(N *p) { -// CHECK-LABEL: _Z2f8P1N -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_N_s_v_x_c:!.*]] - return p->s.v.x.c; -} - -int f9(R *p) { -// CHECK-LABEL: _Z2f9P1R -// CHECK: load i32, i32* {{.*}}, !tbaa [[TAG_R_m_s_v_x_c:!.*]] - return p->m.s.v.x.c; -} - -// CHECK-DAG: [[TAG_U_j]] = !{[[TYPE_U:!.*]], [[TYPE_union_member:!.*]], i64 0} -// CHECK-DAG: [[TAG_S_u_i]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 0} -// CHECK-DAG: [[TAG_S_u_x_b]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 0} -// CHECK-DAG: [[TAG_S_u_x_arr]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 0} -// CHECK-DAG: [[TAG_S_v_j]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 28} -// CHECK-DAG: [[TAG_S_v_x_b]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 28} -// CHECK-DAG: [[TAG_S_v_x_arr]] = !{[[TYPE_S:!.*]], [[TYPE_union_member]], i64 28} -// CHECK-DAG: [[TAG_N_s_v_x_c]] = !{[[TYPE_N:!.*]], [[TYPE_union_member]], i64 0} -// CHECK-DAG: [[TAG_R_m_s_v_x_c]] = !{[[TYPE_R:!.*]], [[TYPE_union_member]], i64 56} -// CHECK-DAG: [[TYPE_U]] = !{!"_ZTS1U", [[TYPE_union_member]], i64 0} -// CHECK-DAG: [[TYPE_S]] = !{!"_ZTS1S", [[TYPE_U]], i64 0, [[TYPE_U]], i64 28} -// CHECK-DAG: [[TYPE_N]] = !{!"_ZTS1N", [[TYPE_union_member]], i64 0} -// CHECK-DAG: [[TYPE_R]] = !{!"_ZTS1R", [[TYPE_N]], i64 0, [[TYPE_N]], i64 56} -// CHECK-DAG: [[TYPE_union_member]] = !{!"union member", [[TYPE_char:!.*]], i64 0} -// CHECK-DAG: [[TYPE_char]] = !{!"omnipotent char", {{.*}}, i64 0} diff --git a/clang/test/CodeGen/union-tbaa1.c b/clang/test/CodeGen/union-tbaa1.c index 7a6d2ccd00d4..07f5fcfeb421 100644 --- a/clang/test/CodeGen/union-tbaa1.c +++ b/clang/test/CodeGen/union-tbaa1.c @@ -15,32 +15,30 @@ void fred(unsigned Num, int Vec[2], int *Index, int Arr[4][2]) { // But no tbaa for the two stores: // CHECK: %uw[[UW1:[0-9]*]] = getelementptr // CHECK: store{{.*}}%uw[[UW1]] -// CHECK: tbaa [[TAG_vect32_union_member:![0-9]+]] +// CHECK: tbaa ![[OCPATH:[0-9]+]] // There will be a load after the store, and it will use tbaa. Make sure // the check-not above doesn't find it: // CHECK: load Tmp[*Index][0].uw = Arr[*Index][0] * Num; // CHECK: %uw[[UW2:[0-9]*]] = getelementptr // CHECK: store{{.*}}%uw[[UW2]] -// CHECK: tbaa [[TAG_vect32_union_member]] +// CHECK: tbaa ![[OCPATH]] Tmp[*Index][1].uw = Arr[*Index][1] * Num; // Same here, don't generate tbaa for the loads: // CHECK: %uh[[UH1:[0-9]*]] = bitcast %union.vect32 // CHECK: %arrayidx[[AX1:[0-9]*]] = getelementptr{{.*}}%uh[[UH1]] // CHECK: load i16, i16* %arrayidx[[AX1]] -// CHECK: tbaa [[TAG_vect32_union_member]] +// CHECK: tbaa ![[OCPATH]] // CHECK: store Vec[0] = Tmp[*Index][0].uh[1]; // CHECK: %uh[[UH2:[0-9]*]] = bitcast %union.vect32 // CHECK: %arrayidx[[AX2:[0-9]*]] = getelementptr{{.*}}%uh[[UH2]] // CHECK: load i16, i16* %arrayidx[[AX2]] -// CHECK: tbaa [[TAG_vect32_union_member]] +// CHECK: tbaa ![[OCPATH]] // CHECK: store Vec[1] = Tmp[*Index][1].uh[1]; bar(Tmp); } -// CHECK-DAG: [[TAG_vect32_union_member]] = !{[[TYPE_vect32:!.*]], [[TYPE_union_member:!.*]], i64 0} -// CHECK-DAG: [[TYPE_vect32]] = !{!"", [[TYPE_union_member]], i64 0} -// CHECK-DAG: [[TYPE_union_member]] = !{!"union member", [[TYPE_char:!.*]], i64 0} -// CHECK-DAG: [[TYPE_char]] = !{!"omnipotent char", {{.*}}} +// CHECK-DAG: ![[CHAR:[0-9]+]] = !{!"omnipotent char" +// CHECK-DAG: ![[OCPATH]] = !{![[CHAR]], ![[CHAR]], i64 0}