r286534 - PR30937: don't devirtualize if we find that the callee is a pure virtual

[Mehdi Amini via cfe-commits]

Thinking again, the method “can” be implemented I think. Forget it…

— 
Mehdi

> On Nov 10, 2016, at 6:11 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> Could we emit llvm.unreachable in this case?
> 
> — 
> Mehdi
> 
>> On Nov 10, 2016, at 5:01 PM, Richard Smith via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>> 
>> Author: rsmith
>> Date: Thu Nov 10 19:01:31 2016
>> New Revision: 286534
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=286534&view=rev
>> Log:
>> PR30937: don't devirtualize if we find that the callee is a pure virtual
>> function. In that case, there is no requirement that the callee is actually
>> defined, and the code may in fact be valid and have defined behavior if the
>> virtual call is unreachable.
>> 
>> Modified:
>>   cfe/trunk/lib/CodeGen/CGClass.cpp
>>   cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=286534&r1=286533&r2=286534&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Nov 10 19:01:31 2016
>> @@ -2851,9 +2851,9 @@ CodeGenFunction::CanDevirtualizeMemberFu
>>    return false;
>> 
>>  // If the member function is marked 'final', we know that it can't be
>> -  // overridden and can therefore devirtualize it.
>> +  // overridden and can therefore devirtualize it unless it's pure virtual.
>>  if (MD->hasAttr<FinalAttr>())
>> -    return true;
>> +    return !MD->isPure();
>> 
>>  // If the base expression (after skipping derived-to-base conversions) is a
>>  // class prvalue, then we can devirtualize.
>> @@ -2861,31 +2861,28 @@ CodeGenFunction::CanDevirtualizeMemberFu
>>  if (Base->isRValue() && Base->getType()->isRecordType())
>>    return true;
>> 
>> -  // If the most derived class is marked final, we know that no subclass can
>> -  // override this member function and so we can devirtualize it. For example:
>> -  //
>> -  // struct A { virtual void f(); }
>> -  // struct B final : A { };
>> -  //
>> -  // void f(B *b) {
>> -  //   b->f();
>> -  // }
>> -  //
>> -  if (const CXXRecordDecl *BestDynamicDecl = Base->getBestDynamicClassType()) {
>> -    if (BestDynamicDecl->hasAttr<FinalAttr>())
>> -      return true;
>> -
>> -    // There may be a method corresponding to MD in a derived class. If that
>> -    // method is marked final, we can devirtualize it.
>> -    const CXXMethodDecl *DevirtualizedMethod =
>> -        MD->getCorrespondingMethodInClass(BestDynamicDecl);
>> -    if (DevirtualizedMethod->hasAttr<FinalAttr>())
>> -      return true;
>> -  }
>> +  // If we don't even know what we would call, we can't devirtualize.
>> +  const CXXRecordDecl *BestDynamicDecl = Base->getBestDynamicClassType();
>> +  if (!BestDynamicDecl)
>> +    return false;
>> +
>> +  // There may be a method corresponding to MD in a derived class.
>> +  const CXXMethodDecl *DevirtualizedMethod =
>> +      MD->getCorrespondingMethodInClass(BestDynamicDecl);
>> +
>> +  // If that method is pure virtual, we can't devirtualize. If this code is
>> +  // reached, the result would be UB, not a direct call to the derived class
>> +  // function, and we can't assume the derived class function is defined.
>> +  if (DevirtualizedMethod->isPure())
>> +    return false;
>> +
>> +  // If that method is marked final, we can devirtualize it.
>> +  if (DevirtualizedMethod->hasAttr<FinalAttr>())
>> +    return true;
>> 
>>  // Similarly, if the class itself is marked 'final' it can't be overridden
>>  // and we can therefore devirtualize the member function call.
>> -  if (MD->getParent()->hasAttr<FinalAttr>())
>> +  if (BestDynamicDecl->hasAttr<FinalAttr>())
>>    return true;
>> 
>>  if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Base)) {
>> 
>> Modified: cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp?rev=286534&r1=286533&r2=286534&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp Thu Nov 10 19:01:31 2016
>> @@ -161,3 +161,37 @@ namespace test4 {
>>    p->fish.eat();
>>  }
>> }
>> +
>> +// Do not devirtualize to pure virtual function calls.
>> +namespace test5 {
>> +  struct X {
>> +    virtual void f() = 0;
>> +  };
>> +  struct Y {};
>> +  // CHECK-LABEL: define {{.*}} @_ZN5test51f
>> +  void f(Y &y, X Y::*p) {
>> +    // CHECK-NOT: call {{.*}} @_ZN5test51X1fEv
>> +    // CHECK: call void %
>> +    (y.*p).f();
>> +  };
>> +
>> +  struct Z final {
>> +    virtual void f() = 0;
>> +  };
>> +  // CHECK-LABEL: define {{.*}} @_ZN5test51g
>> +  void g(Z &z) {
>> +    // CHECK-NOT: call {{.*}} @_ZN5test51Z1fEv
>> +    // CHECK: call void %
>> +    z.f();
>> +  }
>> +
>> +  struct Q {
>> +    virtual void f() final = 0;
>> +  };
>> +  // CHECK-LABEL: define {{.*}} @_ZN5test51h
>> +  void h(Q &q) {
>> +    // CHECK-NOT: call {{.*}} @_ZN5test51Q1fEv
>> +    // CHECK: call void %
>> +    q.f();
>> +  }
>> +}
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>