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

[Mehdi Amini via cfe-commits]

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