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

[Hans Wennborg via cfe-commits]

r281413 for the constructors.

On Tue, Sep 13, 2016 at 2:58 PM, Hans Wennborg <hans at chromium.org> wrote:
> 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
>>