r194000 - Use aliases for more constructors and destructors.

Yaron Keren yaron.keren at gmail.com
Tue Nov 5 10:42:19 PST 2013


The program is rather large, using clang to compile significant C++ code to
IR code and the JIT to execute. I do not know enough to make a small
testcase. Sorry.

Anyhow, the problem started in r194000, in r193999 it's OK.
In r194046 (your patch) the problem is indeed solved.
With latest ToT r194086 the problem is back, probably since r194046 was
reverted.

Yaron



2013/11/5 Rafael EspĂ­ndola <rafael.espindola at gmail.com>

> Can you provide a test case? Could you also try the patch I emailed about
> using the aliasee directly when possible?
>
>
> On Tuesday, November 5, 2013, Yaron Keren wrote:
>
>> Hi,
>>
>> This patch causes
>>
>>  Assertion failed: !isAlreadyCodeGenerating && "Error: Recursive
>> compilation detected!", file ..\..\..\..\lib\ExecutionEngine\JIT\JIT.cpp,
>> line 467
>>
>> and fixed by setting
>>
>>  getCodeGenOpts().CXXCtorDtorAliases = 0;
>>
>> Is this the right fix or should something else in the JIT logic be
>> modified?
>>
>> Yaron
>>
>>
>> 2013/11/4 Rafael Espindola <rafael.espindola at gmail.com>
>>
>>> Author: rafael
>>> Date: Mon Nov  4 12:38:59 2013
>>> New Revision: 194000
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=194000&view=rev
>>> Log:
>>> Use aliases for more constructors and destructors.
>>>
>>> With this patch we produce alias for cases like
>>>
>>> template<typename T>
>>> struct foobar {
>>>   foobar() {
>>>   }
>>> };
>>> template struct foobar<void>;
>>>
>>> We just have to be careful to produce the same aliases in every TU
>>> because
>>> of comdats.
>>>
>>> Added:
>>>     cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp
>>> Modified:
>>>     cfe/trunk/lib/CodeGen/CGCXX.cpp
>>>     cfe/trunk/lib/CodeGen/CGVTables.cpp
>>>     cfe/trunk/lib/CodeGen/CodeGenModule.h
>>>     cfe/trunk/test/CodeGenCXX/destructors.cpp
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGCXX.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXX.cpp?rev=194000&r1=193999&r2=194000&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGCXX.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGCXX.cpp Mon Nov  4 12:38:59 2013
>>> @@ -95,12 +95,16 @@ bool CodeGenModule::TryEmitBaseDestructo
>>>      return true;
>>>
>>>    return TryEmitDefinitionAsAlias(GlobalDecl(D, Dtor_Base),
>>> -                                  GlobalDecl(BaseD, Dtor_Base));
>>> +                                  GlobalDecl(BaseD, Dtor_Base),
>>> +                                  false);
>>>  }
>>>
>>>  /// 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) {
>>> +                                             GlobalDecl TargetDecl,
>>> +                                             bool InEveryTU) {
>>>    if (!getCodeGenOpts().CXXCtorDtorAliases)
>>>      return true;
>>>
>>> @@ -108,34 +112,34 @@ bool CodeGenModule::TryEmitDefinitionAsA
>>>    // support aliases with that linkage, fail.
>>>    llvm::GlobalValue::LinkageTypes Linkage =
>>> getFunctionLinkage(AliasDecl);
>>>
>>> -  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;
>>> -
>>> -  // Other linkages will probably never be supported.
>>> -  default:
>>> -    return true;
>>> -  }
>>> -
>>>    llvm::GlobalValue::LinkageTypes TargetLinkage
>>>      = getFunctionLinkage(TargetDecl);
>>>
>>> -  if (llvm::GlobalValue::isWeakForLinker(TargetLinkage))
>>> +  // Don't create an alias to a linker weak symbol unless we know we
>>> can do
>>> +  // that in every TU. This avoids producing different COMDATs in
>>> different
>>> +  // TUs.
>>> +  if (llvm::GlobalValue::isWeakForLinker(TargetLinkage)) {
>>> +    if (!InEveryTU)
>>> +      return true;
>>> +
>>> +    // In addition to making sure we produce it in every TU, we have to
>>> make
>>> +    // sure llvm keeps it.
>>> +    // FIXME: Instead of outputting an alias we could just replace
>>> every use of
>>> +    // AliasDecl with TargetDecl.
>>> +    assert(Linkage == TargetLinkage);
>>> +    Linkage = llvm::GlobalValue::WeakODRLinkage;
>>> +  }
>>> +
>>> +  // We can't use an alias if the linkage is not valid for one.
>>> +  if (!llvm::GlobalAlias::isValidLinkage(Linkage))
>>>      return true;
>>>
>>> +  // Check if we have it already.
>>> +  StringRef MangledName = getMangledName(AliasDecl);
>>> +  llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
>>> +  if (Entry && !Entry->isDeclaration())
>>> +    return false;
>>> +
>>>    // Derive the type for the alias.
>>>    llvm::PointerType *AliasType
>>>      = getTypes().GetFunctionType(AliasDecl)->getPointerTo();
>>> @@ -153,10 +157,7 @@ bool CodeGenModule::TryEmitDefinitionAsA
>>>      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);
>>> @@ -177,11 +178,14 @@ void CodeGenModule::EmitCXXConstructor(c
>>>    // 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() &&
>>> -      !TryEmitDefinitionAsAlias(GlobalDecl(ctor, Ctor_Complete),
>>> -                                GlobalDecl(ctor, Ctor_Base)))
>>> -    return;
>>> +      (ctorType == Ctor_Complete || ctorType == Ctor_Base)) {
>>> +    bool ProducedAlias =
>>> +        !TryEmitDefinitionAsAlias(GlobalDecl(ctor, Ctor_Complete),
>>> +                                  GlobalDecl(ctor, Ctor_Base), true);
>>> +    if (ctorType == Ctor_Complete && ProducedAlias)
>>> +      return;
>>> +  }
>>>
>>>    const CGFunctionInfo &fnInfo =
>>>      getTypes().arrangeCXXConstructorDeclaration(ctor, ctorType);
>>> @@ -218,11 +222,18 @@ void CodeGenModule::EmitCXXDestructor(co
>>>                                        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 (dtorType == Dtor_Complete &&
>>> -      !dtor->getParent()->getNumVBases() &&
>>> -      !TryEmitDefinitionAsAlias(GlobalDecl(dtor, Dtor_Complete),
>>> -                                GlobalDecl(dtor, Dtor_Base)))
>>> -    return;
>>> +  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));
>>> +    }
>>> +  }
>>>
>>>    // The base destructor is equivalent to the base destructor of its
>>>    // base class if there is exactly one non-virtual base class with a
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=194000&r1=193999&r2=194000&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Mon Nov  4 12:38:59 2013
>>> @@ -387,10 +387,6 @@ void CodeGenVTables::emitThunk(GlobalDec
>>>        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;
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=194000&r1=193999&r2=194000&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
>>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Mon Nov  4 12:38:59 2013
>>> @@ -1034,7 +1034,8 @@ private:
>>>
>>>    // C++ related functions.
>>>
>>> -  bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target);
>>> +  bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
>>> +                                bool InEveryTU);
>>>    bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
>>>
>>>    void EmitNamespace(const NamespaceDecl *D);
>>>
>>> Added: cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp?rev=194000&view=auto
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp (added)
>>> +++ cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp Mon Nov  4 12:38:59
>>> 2013
>>> @@ -0,0 +1,62 @@
>>> +// RUN: %clang_cc1 %s -triple x86_64-linux -emit-llvm -o -
>>> -mconstructor-aliases | FileCheck %s
>>> +
>>> +namespace test1 {
>>> +// test that we produce an alias when the destructor is weak_odr
>>> +
>>> +// CHECK-DAG: @_ZN5test16foobarIvEC1Ev = alias weak_odr void
>>> (%"struct.test1::foobar"*)* @_ZN5test16foobarIvEC2Ev
>>> +// CHECK-DAG: define weak_odr void @_ZN5test16foobarIvEC2Ev(
>>> +template <typename T> struct foobar {
>>> +  foobar() {}
>>> +};
>>> +
>>> +template struct foobar<void>;
>>> +}
>>> +
>>> +namespace test2 {
>>> +// test that we produce an alias when the destrucor is linkonce_odr.
>>> Note that
>>> +// the alias itself is weak_odr to make sure we don't get a translation
>>> unit
>>> +// with just _ZN5test26foobarIvEC2Ev in it.
>>> +
>>> +// CHECK-DAG: @_ZN5test26foobarIvEC1Ev = alias weak_odr void
>>> (%"struct.test2::foobar"*)* @_ZN5test26foobarIvEC2Ev
>>> +// CHECK-DAG: define linkonce_odr void @_ZN5test26foobarIvEC2Ev(
>>> +void g();
>>> +template <typename T> struct foobar {
>>> +  foobar() { g(); }
>>> +};
>>> +foobar<void> x;
>>> +}
>>> +
>>> +namespace test3 {
>>> +// test that these alias are internal.
>>> +
>>> +// CHECK-DAG: @_ZN5test312_GLOBAL__N_11AD1Ev = alias internal void
>>> (%"struct.test3::<anonymous namespace>::A"*)* @_ZN5test312_GLOBAL__N_11AD2Ev
>>> +// CHECK-DAG: @_ZN5test312_GLOBAL__N_11BD2Ev = alias internal bitcast
>>> (void (%"struct.test3::<anonymous namespace>::A"*)*
>>> @_ZN5test312_GLOBAL__N_11AD2Ev to void (%"struct.test3::<anonymous
>>> namespace>::B"*)*)
>>> +// CHECK-DAG: @_ZN5test312_GLOBAL__N_11BD1Ev = alias internal void
>>> (%"struct.test3::<anonymous namespace>::B"*)* @_ZN5test312_GLOBAL__N_11BD2Ev
>>> +// CHECK-DAG: define internal void @_ZN5test312_GLOBAL__N_11AD2Ev(
>>> +namespace {
>>> +struct A {
>>> +  ~A() {}
>>> +};
>>> +
>>> +struct B : public A {};
>>> +}
>>> +
>>> +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.
>>> +
>>> +  // CHECK-DAG: @_ZN5test41BD1Ev = alias weak_odr void
>>> (%"struct.test4::B"*)* @_ZN5test41BD2Ev
>>> +  // CHECK-DAG: define linkonce_odr void @_ZN5test41BD2Ev(
>>> +  // CHECK-DAG: @_ZN5test41AD1Ev = alias weak_odr void
>>> (%"struct.test4::A"*)* @_ZN5test41AD2Ev
>>> +  // CHECK-DAG: define linkonce_odr void @_ZN5test41AD2Ev(
>>> +  struct A {
>>> +    virtual ~A() {}
>>> +  };
>>> +  struct B : public A{
>>> +    ~B() {}
>>> +  };
>>> +  B X;
>>> +}
>>>
>>> Modified: cfe/trunk/test/CodeGenCXX/destructors.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/destructors.cpp?rev=194000&r1=193999&r2=194000&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGenCXX/destructors.cpp (original)
>>> +++ cfe/trunk/test/CodeGenCXX/destructors.cpp Mon Nov  4 12:38:59 2013
>>> @@ -9,6 +9,7 @@
>>>  // 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
>>> +// CHECK-DAG: @_ZN6PR752617allocator_derivedD1Ev = alias weak_odr void
>>> (%"struct.PR7526::allocator_derived"*)* @_ZN6PR752617allocator_derivedD2Ev
>>>
>>>  struct A {
>>>    int a;
>>> @@ -44,9 +45,6 @@ namespace PR7526 {
>>>    // 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;
>>>    }
>>> @@ -396,6 +394,11 @@ namespace test9 {
>>>    // CHECK: call void @_ZN5test31AD2Ev(
>>>    // 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_11CD1Ev(
>>> +  // CHECK: ret void
>>> +
>>>    // CHECK: declare void @_ZN5test31BD2Ev(
>>>    // CHECK: declare void @_ZN5test31AD2Ev(
>>>
>>> @@ -408,11 +411,6 @@ namespace test9 {
>>>    // 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
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131105/1769d014/attachment.html>


More information about the cfe-commits mailing list