Fix a bug with base offset merging that Devang noticed.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@97641 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/CGVtable.cpp

@@ -99,7 +99,8 @@ private:
   /// need to perform return value adjustments.
   AdjustmentOffsetsMapTy ReturnAdjustments;
 
-  typedef llvm::SmallVector<uint64_t, 1> OffsetVectorTy;
+  // FIXME: We might be able to get away with making this a SmallSet.
+  typedef llvm::SmallSetVector<uint64_t, 2> OffsetSetVectorTy;
   
   /// SubobjectOffsetsMapTy - This map is used for keeping track of all the
   /// base subobject offsets that a single class declaration might refer to.
@@ -114,7 +115,7 @@ private:
   /// when we determine that C::f() overrides A::f(), we need to update the
   /// overriders map for both A-in-B1 and A-in-B2 and the subobject offsets map
   /// will have the subobject offsets for both A copies.
-  typedef llvm::DenseMap<const CXXRecordDecl *, OffsetVectorTy>
+  typedef llvm::DenseMap<const CXXRecordDecl *, OffsetSetVectorTy>
     SubobjectOffsetsMapTy;
   
   /// ComputeFinalOverriders - Compute the final overriders for a given base
@@ -360,7 +361,7 @@ void FinalOverriders::PropagateOverrider(const CXXMethodDecl *OldMD,
     /// struct C : B1, B2 { virtual void f(); };
     ///
     /// When overriding A::f with C::f we need to do so in both A subobjects.
-    const OffsetVectorTy &OffsetVector = Offsets[OverriddenRD];
+    const OffsetSetVectorTy &OffsetVector = Offsets[OverriddenRD];
     
     // Go through all the subobjects.
     for (unsigned I = 0, E = OffsetVector.size(); I != E; ++I) {
@@ -404,41 +405,12 @@ FinalOverriders::MergeSubobjectOffsets(const SubobjectOffsetsMapTy &NewOffsets,
   for (SubobjectOffsetsMapTy::const_iterator I = NewOffsets.begin(),
        E = NewOffsets.end(); I != E; ++I) {
     const CXXRecordDecl *NewRD = I->first;
-    const OffsetVectorTy& NewOffsetVector = I->second;
+    const OffsetSetVectorTy& NewOffsetVector = I->second;
     
-    OffsetVectorTy &OffsetVector = Offsets[NewRD];
-    if (OffsetVector.empty()) {
-      // There were no previous offsets in this vector, just insert all entries
-      // from the new offset vector.
-      OffsetVector.append(NewOffsetVector.begin(), NewOffsetVector.end());
-      continue;
-    }
+    OffsetSetVectorTy &OffsetVector = Offsets[NewRD];
     
-    // We need to merge the new offsets vector into the old, but we don't want
-    // to have duplicate entries. Do this by inserting the old offsets in a set
-    // so they'll be unique. After this, we iterate over the new offset vector
-    // and only append elements that aren't in the set.
-    
-    // First, add the existing offsets to the set.
-    llvm::SmallSet<uint64_t, 4> OffsetSet;
-    for (unsigned I = 0, E = OffsetVector.size(); I != E; ++I) {
-      bool Inserted = OffsetSet.insert(OffsetVector[I]);
-      if (!Inserted)
-        assert(false && "Set of offsets should be unique!");
-    }
-    
-    // Next, only add the new offsets if they are not already in the set.
-    for (unsigned I = 0, E = NewOffsetVector.size(); I != E; ++I) {
-      uint64_t Offset = NewOffsetVector[I];
-
-      if (OffsetSet.count(Offset)) {
-        // Ignore the offset.
-        continue;
-      }
-      
-      // Otherwise, add it to the offsets vector.
-      OffsetVector.push_back(Offset);
-    }
+    // Merge the new offsets set vector into the old.
+    OffsetVector.insert(NewOffsetVector.begin(), NewOffsetVector.end());
   }
 }
 
@@ -503,7 +475,7 @@ void FinalOverriders::ComputeFinalOverriders(BaseSubobject Base,
   MergeSubobjectOffsets(NewOffsets, Offsets);
 
   /// Finally, add the offset for our own subobject.
-  Offsets[RD].push_back(Base.getBaseOffset());
+  Offsets[RD].insert(Base.getBaseOffset());
 }
 
 void FinalOverriders::dump(llvm::raw_ostream &Out, BaseSubobject Base) {

test/CodeGenCXX/vtable-layout.cpp

@@ -978,3 +978,61 @@ struct D : B, C {
 void D::f() { }
 
 }
+
+namespace Test25 {
+  
+// This mainly tests that we don't assert on this class hierarchy.
+
+struct V {
+  virtual void f();
+};
+
+struct A : virtual V { };
+struct B : virtual V { };
+
+// CHECK:      Vtable for 'Test25::C' (11 entries).
+// CHECK-NEXT:    0 | vbase_offset (0)
+// CHECK-NEXT:    1 | vcall_offset (0)
+// CHECK-NEXT:    2 | offset_to_top (0)
+// CHECK-NEXT:    3 | Test25::C RTTI
+// CHECK-NEXT:        -- (Test25::A, 0) vtable address --
+// CHECK-NEXT:        -- (Test25::C, 0) vtable address --
+// CHECK-NEXT:        -- (Test25::V, 0) vtable address --
+// CHECK-NEXT:    4 | void Test25::V::f()
+// CHECK-NEXT:    5 | void Test25::C::g()
+// CHECK-NEXT:    6 | vbase_offset (-8)
+// CHECK-NEXT:    7 | vcall_offset (-8)
+// CHECK-NEXT:    8 | offset_to_top (-8)
+// CHECK-NEXT:    9 | Test25::C RTTI
+// CHECK-NEXT:        -- (Test25::B, 8) vtable address --
+// CHECK-NEXT:        -- (Test25::V, 8) vtable address --
+// CHECK-NEXT:   10 | [unused] void Test25::V::f()
+
+// CHECK:      Construction vtable for ('Test25::A', 0) in 'Test25::C' (5 entries).
+// CHECK-NEXT:    0 | vbase_offset (0)
+// CHECK-NEXT:    1 | vcall_offset (0)
+// CHECK-NEXT:    2 | offset_to_top (0)
+// CHECK-NEXT:    3 | Test25::A RTTI
+// CHECK-NEXT:        -- (Test25::A, 0) vtable address --
+// CHECK-NEXT:        -- (Test25::V, 0) vtable address --
+// CHECK-NEXT:    4 | void Test25::V::f()
+
+// CHECK:      Construction vtable for ('Test25::B', 8) in 'Test25::C' (9 entries).
+// CHECK-NEXT:    0 | vbase_offset (-8)
+// CHECK-NEXT:    1 | vcall_offset (-8)
+// CHECK-NEXT:    2 | offset_to_top (0)
+// CHECK-NEXT:    3 | Test25::B RTTI
+// CHECK-NEXT:        -- (Test25::B, 8) vtable address --
+// CHECK-NEXT:        -- (Test25::V, 8) vtable address --
+// CHECK-NEXT:    4 | [unused] void Test25::V::f()
+// CHECK-NEXT:    5 | vcall_offset (0)
+// CHECK-NEXT:    6 | offset_to_top (8)
+// CHECK-NEXT:    7 | Test25::B RTTI
+// CHECK-NEXT:        -- (Test25::V, 0) vtable address --
+// CHECK-NEXT:    8 | void Test25::V::f()
+struct C : A, virtual V, B {
+  virtual void g();
+};
+void C::g() { }
+
+}