[CodeGen][typeid] Emit typeinfo directly if type is known at compile-time

[zequanwu]

Summary

Bug: https://bugs.llvm.org/show_bug.cgi?id=47467

[zequanwu]

Remove unused field.

[hans]

I'm not sure that changing isPotentiallyEvaluated() is the right thing to do. The meaning of that corresponds to text in the standard: https://eel.is/c++draft/expr.typeid#3 so changing it to something that doesn't match the standard exactly seems wrong.

I think it would be safer to do the change purely as an optimization in codegen (maybe we could add a new helper method that could also be used by the warning).

Also, I found another interesting test case: https://godbolt.org/z/aW4ojr

#include <typeinfo>

class B {
public:
  virtual void f();
};

B f();

 const std::type_info& g() {
  return typeid(f());
}

Here MSVC warns about /GR- and emits the call to __RTtypeid even though I think it's not necessary since the type is statically known. Actually, if I understand things correctly, f() shouldn't even be evaluated here since the expression is not a glvalue, and Clang gets that right. So that's a case where our codegen is better than MSVC :-)

[zequanwu]

If the operand of CXXTypeidExpr is already most derived object, no need to look up vtable.

[zequanwu]

In D87425#2265523, @hans wrote:

I'm not sure that changing isPotentiallyEvaluated() is the right thing to do. The meaning of that corresponds to text in the standard: https://eel.is/c++draft/expr.typeid#3 so changing it to something that doesn't match the standard exactly seems wrong.

If the type of the operand is already most derived, we could just emit the typeinfo of the type. This would match the standard: https://eel.is/c++draft/expr.typeid#2

I think it would be safer to do the change purely as an optimization in codegen (maybe we could add a new helper method that could also be used by the warning).

For "optimization in codegen", do you mean optimization after the IR is generated or like I did in CodeGenFunction::EmitCXXTypeidExpr?

[hans]

I think it would be safer to do the change purely as an optimization in codegen (maybe we could add a new helper method that could also be used by the warning).

For "optimization in codegen", do you mean optimization after the IR is generated or like I did in CodeGenFunction::EmitCXXTypeidExpr?

I mean like you did in CodeGenFunction::EmitCXXTypeidExpr.

clang/include/clang/AST/ExprCXX.h

861

A comment would be good here.

clang/lib/AST/ExprCXX.cpp

151

I suppose it depends on what you want the semantics of this function to be, but returning false here seems odd.. wouldn't true make as much sense?
If this function is not intended to be called for type operands, maybe assert about that instead?

154

I think checking for DeclRefExpr isn't guaranteed to handle all cases, like

typeid(*&obj)

for example. That's okay, but it should be clear from the function comment (and maybe the name) that this is a best-effort check, not a strong guarantee about whether the expression refers to a most derived object.

clang/test/SemaCXX/constant-expression-cxx2a.cpp

312 ↗

(On Diff #291619)

This appears to be changing semantics. It could be that this test is unnecessary strict (that's my understanding), but usually these checks are based on examples in the standard, or the current understanding of the standard. I think it would be best to check with Richard before changing this.

Actually, I'm surprised this affected since you're only updating CodeGen, not Sema. Is the static_assert really invoking Codegen?

[zequanwu]

Address comment.

[zequanwu]

clang/test/SemaCXX/constant-expression-cxx2a.cpp

312 ↗

(On Diff #291619)

This was caused by my previous changes on isPotentiallyEvaluated, which is used by Sema. I reverted this.

[hans]

lgtm

clang/test/SemaCXX/constant-expression-cxx2a.cpp

312 ↗

(On Diff #291619)

Ah, great. That makes more sense :)

clang/include/clang/AST/ExprCXX.h

CXXTypeidExpr(EmptyShell Empty, bool isExpr)
     else
       Operand = (TypeSourceInfo*)nullptr;
   }
 
   /// Determine whether this typeid has a type operand which is potentially
   /// evaluated, per C++11 [expr.typeid]p3.
   bool isPotentiallyEvaluated() const;
 
+  /// Best-effort check if the expression operand refers to a most derived
+  /// object. This is not a strong guarantee.
+  bool isMostDerived(ASTContext &Context) const;
+
   bool isTypeOperand() const { return Operand.is<TypeSourceInfo *>(); }
 
   /// Retrieves the type operand of this typeid() expression after
   /// various required adjustments (removing reference types, cv-qualifiers).
   QualType getTypeOperand(ASTContext &Context) const;
 
   /// Retrieve source information for the type operand.
   TypeSourceInfo *getTypeOperandSourceInfo() const {

clang/lib/AST/ExprCXX.cpp

bool CXXTypeidExpr::isPotentiallyEvaluated() const {
   const Expr *E = getExprOperand();
   if (const CXXRecordDecl *RD = E->getType()->getAsCXXRecordDecl())
     if (RD->isPolymorphic() && E->isGLValue())
       return true;
 
   return false;
 }
 
+bool CXXTypeidExpr::isMostDerived(ASTContext &Context) const {
+  assert(!isTypeOperand() && "Cannot call isMostDerived for typeid(type)");
+  const Expr *E = getExprOperand()->IgnoreParenNoopCasts(Context);
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+    QualType Ty = DRE->getDecl()->getType();
+    if (!Ty->isPointerType() && !Ty->isReferenceType())
+      return true;
+  }
+
+  return false;
+}
+
 QualType CXXTypeidExpr::getTypeOperand(ASTContext &Context) const {
   assert(isTypeOperand() && "Cannot call getTypeOperand for typeid(expr)");
   Qualifiers Quals;
   return Context.getUnqualifiedArrayType(
       Operand.get<TypeSourceInfo *>()->getType().getNonReferenceType(), Quals);
 }
 
 QualType CXXUuidofExpr::getTypeOperand(ASTContext &Context) const {

clang/lib/CodeGen/CGExprCXX.cpp

if (E->isTypeOperand()) {
     return Builder.CreateBitCast(TypeInfo, StdTypeInfoPtrTy);
   }
 
   // C++ [expr.typeid]p2:
   //   When typeid is applied to a glvalue expression whose type is a
   //   polymorphic class type, the result refers to a std::type_info object
   //   representing the type of the most derived object (that is, the dynamic
   //   type) to which the glvalue refers.
-  if (E->isPotentiallyEvaluated())
+  // If the operand is already most derived object, no need to look up vtable.
+  if (E->isPotentiallyEvaluated() && !E->isMostDerived(getContext()))
     return EmitTypeidFromVTable(*this, E->getExprOperand(),
                                 StdTypeInfoPtrTy);
 
   QualType OperandTy = E->getExprOperand()->getType();
   return Builder.CreateBitCast(CGM.GetAddrOfRTTIDescriptor(OperandTy),
                                StdTypeInfoPtrTy);
 }
 

clang/test/CodeGenCXX/microsoft-abi-typeid.cpp

 // CHECK-NEXT:   ret %struct.type_info* [[RET]]
 
 const std::type_info* test4_typeid() { return &typeid(b); }
 // CHECK: define dso_local %struct.type_info* @"?test4_typeid@@YAPBUtype_info@@XZ"()
 // CHECK:   ret %struct.type_info* bitcast (%rtti.TypeDescriptor2* @"??_R0H@8" to %struct.type_info*)
 
 const std::type_info* test5_typeid() { return &typeid(v); }
 // CHECK: define dso_local %struct.type_info* @"?test5_typeid@@YAPBUtype_info@@XZ"()
-// CHECK:        [[RT:%.*]] = call i8* @__RTtypeid(i8* bitcast (%struct.V* @"?v@@3UV@@A" to i8*))
-// CHECK-NEXT:   [[RET:%.*]] = bitcast i8* [[RT]] to %struct.type_info*
-// CHECK-NEXT:   ret %struct.type_info* [[RET]]
+// CHECK:   ret %struct.type_info* bitcast (%rtti.TypeDescriptor7* @"??_R0?AUV@@@8" to %struct.type_info*)
+
+const std::type_info *test6_typeid() { return &typeid((V &)v); }
+// CHECK: define dso_local %struct.type_info* @"?test6_typeid@@YAPBUtype_info@@XZ"()
+// CHECK:   ret %struct.type_info* bitcast (%rtti.TypeDescriptor7* @"??_R0?AUV@@@8" to %struct.type_info*)
 
 namespace PR26329 {
 struct Polymorphic {
   virtual ~Polymorphic();
 };
 
 void f(const Polymorphic &poly) {
   try {