r281395 - Try harder to not inline dllimport functions referencing non-dllimport functions

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 13 14:58:29 PDT 2016


Good point. Constructors are also a problem, I'll try to fix.

It's not exactly the same issue, because they do show up in the AST,
but they're referenced with a CXXConstructExpr, and not the
DeclRefExpr which DLLImportFunctionVisitor is looking for.

There doesn't seem to be any problem with operator= though.

On Tue, Sep 13, 2016 at 2:36 PM, Nico Weber via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Could other implicit functions (operator=, ctors) have similar issues?
>
> On Tue, Sep 13, 2016 at 5:08 PM, Hans Wennborg via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> Author: hans
>> Date: Tue Sep 13 16:08:20 2016
>> New Revision: 281395
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=281395&view=rev
>> Log:
>> Try harder to not inline dllimport functions referencing non-dllimport
>> functions
>>
>> In r246338, code was added to check for this, but it failed to take into
>> account implicit destructor invocations because those are not reflected
>> in the AST. This adds a separate check for them.
>>
>> Modified:
>>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>>     cfe/trunk/test/CodeGenCXX/dllimport-members.cpp
>>     cfe/trunk/test/CodeGenCXX/dllimport.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=281395&r1=281394&r2=281395&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Sep 13 16:08:20 2016
>> @@ -1740,8 +1740,17 @@ CodeGenModule::isTriviallyRecursive(cons
>>    return Walker.Result;
>>  }
>>
>> -bool
>> -CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
>> +// Check if T is a class type with a destructor that's not dllimport.
>> +static bool HasNonDllImportDtor(QualType T) {
>> +  if (const RecordType *RT = dyn_cast<RecordType>(T))
>> +    if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl()))
>> +      if (RD->getDestructor() &&
>> !RD->getDestructor()->hasAttr<DLLImportAttr>())
>> +        return true;
>> +
>> +  return false;
>> +}
>> +
>> +bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
>>    if (getFunctionLinkage(GD) !=
>> llvm::Function::AvailableExternallyLinkage)
>>      return true;
>>    const auto *F = cast<FunctionDecl>(GD.getDecl());
>> @@ -1754,6 +1763,18 @@ CodeGenModule::shouldEmitFunction(Global
>>      Visitor.TraverseFunctionDecl(const_cast<FunctionDecl*>(F));
>>      if (!Visitor.SafeToInline)
>>        return false;
>> +
>> +    if (const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(F)) {
>> +      // Implicit destructor invocations aren't captured in the AST, so
>> the
>> +      // check above can't see them. Check for them manually here.
>> +      for (const Decl *Member : Dtor->getParent()->decls())
>> +        if (isa<FieldDecl>(Member))
>> +          if (HasNonDllImportDtor(cast<FieldDecl>(Member)->getType()))
>> +            return false;
>> +      for (const CXXBaseSpecifier &B : Dtor->getParent()->bases())
>> +        if (HasNonDllImportDtor(B.getType()))
>> +          return false;
>> +    }
>>    }
>>
>>    // PR9614. Avoid cases where the source code is lying to us. An
>> available
>>
>> Modified: cfe/trunk/test/CodeGenCXX/dllimport-members.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-members.cpp?rev=281395&r1=281394&r2=281395&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/dllimport-members.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/dllimport-members.cpp Tue Sep 13 16:08:20
>> 2016
>> @@ -44,7 +44,7 @@ void useSpecials() {
>>  }
>>
>>  // Used to force non-trivial special members.
>> -struct ForceNonTrivial {
>> +struct __declspec(dllimport) ForceNonTrivial {
>>    ForceNonTrivial();
>>    ~ForceNonTrivial();
>>    ForceNonTrivial(const ForceNonTrivial&);
>>
>> Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=281395&r1=281394&r2=281395&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Tue Sep 13 16:08:20 2016
>> @@ -347,6 +347,13 @@ __declspec(dllimport) inline int *Refere
>>  // MO1-DAG: define available_externally dllimport i32*
>> @"\01?ReferencingImportedDelete@@YAPAHXZ"
>>  USE(ReferencingImportedNew)
>>  USE(ReferencingImportedDelete)
>> +struct ClassWithDtor { ~ClassWithDtor() {} };
>> +struct __declspec(dllimport) ClassWithNonDllImportField { ClassWithDtor
>> t; };
>> +struct __declspec(dllimport) ClassWithNonDllImportBase : public
>> ClassWithDtor { };
>> +USECLASS(ClassWithNonDllImportField);
>> +USECLASS(ClassWithNonDllImportBase);
>> +// MO1-DAG: declare dllimport x86_thiscallcc void
>> @"\01??1ClassWithNonDllImportBase@@QAE at XZ"(%struct.ClassWithNonDllImportBase*)
>> +// MO1-DAG: declare dllimport x86_thiscallcc void
>> @"\01??1ClassWithNonDllImportField@@QAE at XZ"(%struct.ClassWithNonDllImportField*)
>>
>>  // A dllimport function with a TLS variable must not be
>> available_externally.
>>  __declspec(dllimport) inline void FunctionWithTLSVar() { static __thread
>> int x = 42; }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


More information about the cfe-commits mailing list