On 12 November 2013 16:59, Rafael Espíndola <[email protected]> wrote:
> Can you try the attached patch?
Since it was most certainly fixing a bug, I committed it as r194520.
If you still have problems, the attached patch reverts the logic all
the way back to never using rauw and using alias only on strong
symbols. It is IMHO the correct way to revert this if needed.
Cheers,
Rafael
diff --git a/lib/CodeGen/CGCXX.cpp b/lib/CodeGen/CGCXX.cpp
index b16c59d..e7ec147 100644
--- a/lib/CodeGen/CGCXX.cpp
+++ b/lib/CodeGen/CGCXX.cpp
@@ -82,23 +82,25 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
if (!UniqueBase)
return true;
+ /// If we don't have a definition for the destructor yet, don't
+ /// emit. We can't emit aliases to declarations; that's just not
+ /// how aliases work.
+ const CXXDestructorDecl *BaseD = UniqueBase->getDestructor();
+ if (!BaseD->isImplicit() && !BaseD->hasBody())
+ return true;
+
// If the base is at a non-zero offset, give up.
const ASTRecordLayout &ClassLayout = Context.getASTRecordLayout(Class);
if (!ClassLayout.getBaseClassOffset(UniqueBase).isZero())
return true;
- const CXXDestructorDecl *BaseD = UniqueBase->getDestructor();
return TryEmitDefinitionAsAlias(GlobalDecl(D, Dtor_Base),
- GlobalDecl(BaseD, Dtor_Base),
- false);
+ GlobalDecl(BaseD, Dtor_Base));
}
/// Try to emit a definition as a global alias for another definition.
-/// If \p InEveryTU is true, we know that an equivalent alias can be produced
-/// in every translation unit.
bool CodeGenModule::TryEmitDefinitionAsAlias(GlobalDecl AliasDecl,
- GlobalDecl TargetDecl,
- bool InEveryTU) {
+ GlobalDecl TargetDecl) {
if (!getCodeGenOpts().CXXCtorDtorAliases)
return true;
@@ -106,20 +108,33 @@ bool CodeGenModule::TryEmitDefinitionAsAlias(GlobalDecl AliasDecl,
// support aliases with that linkage, fail.
llvm::GlobalValue::LinkageTypes Linkage = getFunctionLinkage(AliasDecl);
- // We can't use an alias if the linkage is not valid for one.
- if (!llvm::GlobalAlias::isValidLinkage(Linkage))
+ switch (Linkage) {
+ // We can definitely emit aliases to definitions with external linkage.
+ case llvm::GlobalValue::ExternalLinkage:
+ case llvm::GlobalValue::ExternalWeakLinkage:
+ break;
+
+ // Same with local linkage.
+ case llvm::GlobalValue::InternalLinkage:
+ case llvm::GlobalValue::PrivateLinkage:
+ case llvm::GlobalValue::LinkerPrivateLinkage:
+ break;
+
+ // We should try to support linkonce linkages.
+ case llvm::GlobalValue::LinkOnceAnyLinkage:
+ case llvm::GlobalValue::LinkOnceODRLinkage:
return true;
- llvm::GlobalValue::LinkageTypes TargetLinkage =
- getFunctionLinkage(TargetDecl);
+ // Other linkages will probably never be supported.
+ default:
+ return true;
+ }
- // Check if we have it already.
- StringRef MangledName = getMangledName(AliasDecl);
- llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
- if (Entry && !Entry->isDeclaration())
- return false;
- if (Replacements.count(MangledName))
- return false;
+ llvm::GlobalValue::LinkageTypes TargetLinkage
+ = getFunctionLinkage(TargetDecl);
+
+ if (llvm::GlobalValue::isWeakForLinker(TargetLinkage))
+ return true;
// Derive the type for the alias.
llvm::PointerType *AliasType
@@ -133,34 +148,15 @@ bool CodeGenModule::TryEmitDefinitionAsAlias(GlobalDecl AliasDecl,
if (Ref->getType() != AliasType)
Aliasee = llvm::ConstantExpr::getBitCast(Ref, AliasType);
- // Instead of creating as alias to a linkonce_odr, replace all of the uses
- // of the aliassee.
- if (llvm::GlobalValue::isDiscardableIfUnused(Linkage)) {
- Replacements[MangledName] = Aliasee;
- return false;
- }
-
- if (!InEveryTU) {
- /// If we don't have a definition for the destructor yet, don't
- /// emit. We can't emit aliases to declarations; that's just not
- /// how aliases work.
- if (Ref->isDeclaration())
- return true;
- }
-
- // Don't create an alias to a linker weak symbol. This avoids producing
- // different COMDATs in different TUs. Another option would be to
- // output the alias both for weak_odr and linkonce_odr, but that
- // requires explicit comdat support in the IL.
- if (llvm::GlobalValue::isWeakForLinker(TargetLinkage))
- return true;
-
// Create the alias with no name.
llvm::GlobalAlias *Alias =
new llvm::GlobalAlias(AliasType, Linkage, "", Aliasee, &getModule());
// Switch any previous uses to the alias.
+ StringRef MangledName = getMangledName(AliasDecl);
+ llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
if (Entry) {
+ assert(Entry->isDeclaration() && "definition already exists for alias");
assert(Entry->getType() == AliasType &&
"declaration exists with different type");
Alias->takeName(Entry);
@@ -181,14 +177,11 @@ void CodeGenModule::EmitCXXConstructor(const CXXConstructorDecl *ctor,
// The complete constructor is equivalent to the base constructor
// for classes with no virtual bases. Try to emit it as an alias.
if (getTarget().getCXXABI().hasConstructorVariants() &&
+ ctorType == Ctor_Complete &&
!ctor->getParent()->getNumVBases() &&
- (ctorType == Ctor_Complete || ctorType == Ctor_Base)) {
- bool ProducedAlias =
- !TryEmitDefinitionAsAlias(GlobalDecl(ctor, Ctor_Complete),
- GlobalDecl(ctor, Ctor_Base), true);
- if (ctorType == Ctor_Complete && ProducedAlias)
- return;
- }
+ !TryEmitDefinitionAsAlias(GlobalDecl(ctor, Ctor_Complete),
+ GlobalDecl(ctor, Ctor_Base)))
+ return;
const CGFunctionInfo &fnInfo =
getTypes().arrangeCXXConstructorDeclaration(ctor, ctorType);
@@ -225,18 +218,11 @@ void CodeGenModule::EmitCXXDestructor(const CXXDestructorDecl *dtor,
CXXDtorType dtorType) {
// The complete destructor is equivalent to the base destructor for
// classes with no virtual bases, so try to emit it as an alias.
- if (!dtor->getParent()->getNumVBases() &&
- (dtorType == Dtor_Complete || dtorType == Dtor_Base)) {
- bool ProducedAlias =
- !TryEmitDefinitionAsAlias(GlobalDecl(dtor, Dtor_Complete),
- GlobalDecl(dtor, Dtor_Base), true);
- if (ProducedAlias) {
- if (dtorType == Dtor_Complete)
- return;
- if (dtor->isVirtual())
- getVTables().EmitThunks(GlobalDecl(dtor, Dtor_Complete));
- }
- }
+ if (dtorType == Dtor_Complete &&
+ !dtor->getParent()->getNumVBases() &&
+ !TryEmitDefinitionAsAlias(GlobalDecl(dtor, Dtor_Complete),
+ GlobalDecl(dtor, Dtor_Base)))
+ return;
// The base destructor is equivalent to the base destructor of its
// base class if there is exactly one non-virtual base class with a
diff --git a/lib/CodeGen/CGVTables.cpp b/lib/CodeGen/CGVTables.cpp
index 5d657b0..cbc1a58 100644
--- a/lib/CodeGen/CGVTables.cpp
+++ b/lib/CodeGen/CGVTables.cpp
@@ -387,6 +387,10 @@ void CodeGenVTables::emitThunk(GlobalDecl GD, const ThunkInfo &Thunk,
return;
}
+ // If a function has a body, it should have available_externally linkage.
+ assert(ThunkFn->hasAvailableExternallyLinkage() &&
+ "Function should have available_externally linkage!");
+
// Change the linkage.
CGM.setFunctionLinkage(GD, ThunkFn);
return;
diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index 1f2f4bb..6abbb7d 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -172,34 +172,6 @@ void CodeGenModule::createCUDARuntime() {
CUDARuntime = CreateNVCUDARuntime(*this);
}
-void CodeGenModule::applyReplacements() {
- for (ReplacementsTy::iterator I = Replacements.begin(),
- E = Replacements.end();
- I != E; ++I) {
- StringRef MangledName = I->first();
- llvm::Constant *Replacement = I->second;
- llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
- if (!Entry)
- continue;
- llvm::Function *OldF = cast<llvm::Function>(Entry);
- llvm::Function *NewF = dyn_cast<llvm::Function>(Replacement);
- if (!NewF) {
- llvm::ConstantExpr *CE = cast<llvm::ConstantExpr>(Replacement);
- assert(CE->getOpcode() == llvm::Instruction::BitCast ||
- CE->getOpcode() == llvm::Instruction::GetElementPtr);
- NewF = dyn_cast<llvm::Function>(CE->getOperand(0));
- }
-
- // Replace old with new, but keep the old order.
- OldF->replaceAllUsesWith(Replacement);
- if (NewF) {
- NewF->removeFromParent();
- OldF->getParent()->getFunctionList().insertAfter(OldF, NewF);
- }
- OldF->eraseFromParent();
- }
-}
-
void CodeGenModule::checkAliases() {
bool Error = false;
for (std::vector<GlobalDecl>::iterator I = Aliases.begin(),
@@ -235,7 +207,6 @@ void CodeGenModule::checkAliases() {
void CodeGenModule::Release() {
EmitDeferred();
- applyReplacements();
checkAliases();
EmitCXXGlobalInitFunc();
EmitCXXGlobalDtorFunc();
diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h
index c161224..79c7eb5 100644
--- a/lib/CodeGen/CodeGenModule.h
+++ b/lib/CodeGen/CodeGenModule.h
@@ -279,9 +279,6 @@ class CodeGenModule : public CodeGenTypeCache {
/// is defined once we get to the end of the of the translation unit.
std::vector<GlobalDecl> Aliases;
- typedef llvm::StringMap<llvm::TrackingVH<llvm::Constant> > ReplacementsTy;
- ReplacementsTy Replacements;
-
/// DeferredVTables - A queue of (optional) vtables to consider emitting.
std::vector<const CXXRecordDecl*> DeferredVTables;
@@ -1047,8 +1044,7 @@ private:
// C++ related functions.
- bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
- bool InEveryTU);
+ bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target);
bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
void EmitNamespace(const NamespaceDecl *D);
@@ -1095,9 +1091,6 @@ private:
/// was deferred.
void EmitDeferred();
- /// Call replaceAllUsesWith on all pairs in Replacements.
- void applyReplacements();
-
void checkAliases();
/// EmitDeferredVTables - Emit any vtables which we deferred and
diff --git a/test/CodeGenCXX/ctor-dtor-alias.cpp b/test/CodeGenCXX/ctor-dtor-alias.cpp
index 266eecc..06f3ee3 100644
--- a/test/CodeGenCXX/ctor-dtor-alias.cpp
+++ b/test/CodeGenCXX/ctor-dtor-alias.cpp
@@ -17,11 +17,11 @@ template struct foobar<void>;
}
namespace test2 {
-// test that when the destrucor is linkonce_odr we just replace every use of
-// C1 with C2.
+// FIXME when the constructor is linkonce_odr we should just replace every use
+// of C1 with C2.
+// CHECK-DAG: define linkonce_odr void @_ZN5test26foobarIvEC1Ev(
// CHECK-DAG: define linkonce_odr void @_ZN5test26foobarIvEC2Ev(
-// CHECK-DAG: call void @_ZN5test26foobarIvEC2Ev
void g();
template <typename T> struct foobar {
foobar() { g(); }
@@ -30,11 +30,11 @@ foobar<void> x;
}
namespace test3 {
-// test that instead of an internal alias we just use the other destructor
+// FIXME: Instead of an internal alias we shouldjust use the other destructor
// directly.
-// CHECK-DAG: define internal void @_ZN5test312_GLOBAL__N_11AD2Ev(
-// CHECK-DAG: call i32 @__cxa_atexit{{.*}}_ZN5test312_GLOBAL__N_11AD2Ev
+// CHECK-DAG: @_ZN5test312_GLOBAL__N_11BD1Ev = alias internal
+// CHECK-DAG: call i32 @__cxa_atexit{{.*}}@_ZN5test312_GLOBAL__N_11BD1Ev
namespace {
struct A {
~A() {}
@@ -48,11 +48,12 @@ B x;
namespace test4 {
// Test that we don't produce aliases from B to A. We cannot because we cannot
- // guarantee that they will be present in every TU. Instead, we just call
- // A's destructor directly.
+ // guarantee that they will be present in every TU.
- // CHECK-DAG: define linkonce_odr void @_ZN5test41AD2Ev(
- // CHECK-DAG: call i32 @__cxa_atexit{{.*}}_ZN5test41AD2Ev
+ // FIXME: Instead, we should just call A's destructor directly.
+
+ // CHECK-DAG: define linkonce_odr void @_ZN5test41BD1Ev
+ // CHECK-DAG: call i32 @__cxa_atexit{{.*}}@_ZN5test41BD1Ev
struct A {
virtual ~A() {}
};
@@ -65,8 +66,8 @@ namespace test4 {
namespace test5 {
// similar to test4, but with an internal B.
- // CHECK-DAG: define linkonce_odr void @_ZN5test51AD2Ev(
- // CHECK-DAG: call i32 @__cxa_atexit{{.*}}_ZN5test51AD2Ev
+ // CHECK-DAG: @_ZN5test512_GLOBAL__N_11BD1Ev = alias internal
+ // CHECK-DAG: call i32 @__cxa_atexit{{.*}}@_ZN5test512_GLOBAL__N_11BD1Ev
struct A {
virtual ~A() {}
};
@@ -79,8 +80,8 @@ namespace test5 {
}
namespace test6 {
- // Test that we use ~A directly, even when ~A is not defined. The symbol for
- // ~B would have been internal and still contain a reference to ~A.
+ // FIXME: We should use ~A directly, even when ~A is not defined. The symbol
+ // for ~B would have been internal and still contain a reference to ~A.
struct A {
virtual ~A();
};
@@ -90,7 +91,9 @@ namespace test6 {
};
}
B X;
- // CHECK-DAG: call i32 @__cxa_atexit({{.*}}@_ZN5test61AD2Ev
+ // CHECK-DAG: define internal void @_ZN5test612_GLOBAL__N_11BD2Ev
+ // CHECK-DAG: @_ZN5test612_GLOBAL__N_11BD1Ev = alias internal {{.*}} @_ZN5test612_GLOBAL__N_11BD2Ev
+ // CHECK-DAG: call i32 @__cxa_atexit({{.*}}@_ZN5test612_GLOBAL__N_11BD1Ev
}
namespace test7 {
@@ -109,9 +112,9 @@ namespace test7 {
}
namespace test8 {
- // Test that we replace ~zed with ~bar which is an alias to ~foo.
- // CHECK-DAG: call i32 @__cxa_atexit({{.*}}@_ZN5test83barD2Ev
- // CHECK-DAG: @_ZN5test83barD2Ev = alias {{.*}} @_ZN5test83fooD2Ev
+ // FIXME: We should replace ~zed with ~bar which is an alias to ~foo.
+ // CHECK-DAG: call i32 @__cxa_atexit({{.*}}@_ZN5test83zedD1Ev
+ // CHECK-DAG: define linkonce_odr void @_ZN5test83zedD1Ev
struct foo {
~foo();
};
diff --git a/test/CodeGenCXX/destructors.cpp b/test/CodeGenCXX/destructors.cpp
index 59c97b7..5152fa9 100644
--- a/test/CodeGenCXX/destructors.cpp
+++ b/test/CodeGenCXX/destructors.cpp
@@ -6,6 +6,10 @@
// CHECK-DAG: @_ZN5test11OD2Ev = alias {{.*}} @_ZN5test11AD2Ev
// CHECK-DAG: @_ZN5test11SD2Ev = alias bitcast {{.*}} @_ZN5test11AD2Ev
+// CHECK-DAG: @_ZN5test312_GLOBAL__N_11DD1Ev = alias internal {{.*}} @_ZN5test312_GLOBAL__N_11DD2Ev
+// CHECK-DAG: @_ZN5test312_GLOBAL__N_11DD2Ev = alias internal bitcast {{.*}} @_ZN5test312_GLOBAL__N_11CD2Ev
+// CHECK-DAG: @_ZN5test312_GLOBAL__N_11CD1Ev = alias internal {{.*}} @_ZN5test312_GLOBAL__N_11CD2Ev
+
struct A {
int a;
@@ -36,13 +40,13 @@ namespace PR7526 {
struct allocator_derived : allocator { };
- // CHECK-LABEL: define void @_ZN6PR75263fooEv()
- // CHECK: call void {{.*}} @_ZN6PR75269allocatorD2Ev
-
// CHECK-LABEL: define void @_ZN6PR75269allocatorD2Ev(%"struct.PR7526::allocator"* %this) unnamed_addr
// CHECK: call void @__cxa_call_unexpected
allocator::~allocator() throw() { foo(); }
+ // CHECK-LABEL: define linkonce_odr void @_ZN6PR752617allocator_derivedD1Ev(%"struct.PR7526::allocator_derived"* %this) unnamed_addr
+ // CHECK-NOT: call void @__cxa_call_unexpected
+ // CHECK: }
void foo() {
allocator_derived ad;
}
@@ -180,6 +184,12 @@ namespace test3 {
void test() {
new D; // Force emission of D's vtable
}
+
+ // Checked at top of file:
+ // @_ZN5test312_GLOBAL__N_11CD1Ev = alias internal {{.*}} @_ZN5test312_GLOBAL__N_11CD2Ev
+
+ // More checks at end of file.
+
}
namespace test4 {
@@ -375,7 +385,7 @@ namespace test10 {
// Checks from test3:
// CHECK-LABEL: define internal void @_ZN5test312_GLOBAL__N_11DD0Ev(%"struct.test3::<anonymous namespace>::D"* %this) unnamed_addr
- // CHECK: invoke void {{.*}} @_ZN5test312_GLOBAL__N_11CD2Ev
+ // CHECK: invoke void @_ZN5test312_GLOBAL__N_11DD1Ev(
// CHECK: call void @_ZdlPv({{.*}}) [[NUW:#[0-9]+]]
// CHECK: ret void
// CHECK: landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
@@ -383,9 +393,13 @@ namespace test10 {
// CHECK: call void @_ZdlPv({{.*}}) [[NUW]]
// CHECK: resume { i8*, i32 }
+ // Checked at top of file:
+ // @_ZN5test312_GLOBAL__N_11DD1Ev = alias internal {{.*}} @_ZN5test312_GLOBAL__N_11DD2Ev
+ // @_ZN5test312_GLOBAL__N_11DD2Ev = alias internal bitcast {{.*}} @_ZN5test312_GLOBAL__N_11CD2Ev
+
// CHECK-LABEL: define internal void @_ZThn8_N5test312_GLOBAL__N_11DD1Ev(
// CHECK: getelementptr inbounds i8* {{.*}}, i64 -8
- // CHECK: call void {{.*}} @_ZN5test312_GLOBAL__N_11CD2Ev
+ // CHECK: call void @_ZN5test312_GLOBAL__N_11DD1Ev(
// CHECK: ret void
// CHECK-LABEL: define internal void @_ZThn8_N5test312_GLOBAL__N_11DD0Ev(
@@ -393,11 +407,6 @@ namespace test10 {
// CHECK: call void @_ZN5test312_GLOBAL__N_11DD0Ev(
// CHECK: ret void
- // CHECK-LABEL: define internal void @_ZThn8_N5test312_GLOBAL__N_11CD1Ev(
- // CHECK: getelementptr inbounds i8* {{.*}}, i64 -8
- // CHECK: call void @_ZN5test312_GLOBAL__N_11CD2Ev(
- // CHECK: ret void
-
// CHECK-LABEL: define internal void @_ZN5test312_GLOBAL__N_11CD2Ev(%"struct.test3::<anonymous namespace>::C"* %this) unnamed_addr
// CHECK: invoke void @_ZN5test31BD2Ev(
// CHECK: call void @_ZN5test31AD2Ev(
@@ -407,7 +416,7 @@ namespace test10 {
// CHECK: declare void @_ZN5test31AD2Ev(
// CHECK-LABEL: define internal void @_ZN5test312_GLOBAL__N_11CD0Ev(%"struct.test3::<anonymous namespace>::C"* %this) unnamed_addr
- // CHECK: invoke void @_ZN5test312_GLOBAL__N_11CD2Ev(
+ // CHECK: invoke void @_ZN5test312_GLOBAL__N_11CD1Ev(
// CHECK: call void @_ZdlPv({{.*}}) [[NUW]]
// CHECK: ret void
// CHECK: landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
@@ -415,6 +424,11 @@ namespace test10 {
// CHECK: call void @_ZdlPv({{.*}}) [[NUW]]
// CHECK: resume { i8*, i32 }
+ // CHECK-LABEL: define internal void @_ZThn8_N5test312_GLOBAL__N_11CD1Ev(
+ // CHECK: getelementptr inbounds i8* {{.*}}, i64 -8
+ // CHECK: call void @_ZN5test312_GLOBAL__N_11CD1Ev(
+ // CHECK: ret void
+
// CHECK-LABEL: define internal void @_ZThn8_N5test312_GLOBAL__N_11CD0Ev(
// CHECK: getelementptr inbounds i8* {{.*}}, i64 -8
// CHECK: call void @_ZN5test312_GLOBAL__N_11CD0Ev(
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
