justinstitt created this revision.
Herald added a subscriber: pengfei.
Herald added a project: All.
justinstitt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When building the Linux Kernel (x86_64) CheckICE is a particularly slow
method which results in 0.815% of all cpu cycles (~63 million cycles).
Much of this is due to repeat checks regarding the ICEKind of binary
expressions.

Storing ICEKind information within BinaryOperator and stopping future
CheckICE calls before they do repeat work shows a net speed increase of
~80% for CheckICE (~14 million cycles).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131447

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ExprConstant.cpp

Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -15285,17 +15285,6 @@
 
 namespace {
 
-enum ICEKind {
-  /// This expression is an ICE.
-  IK_ICE,
-  /// This expression is not an ICE, but if it isn't evaluated, it's
-  /// a legal subexpression for an ICE. This return value is used to handle
-  /// the comma operator in C99 mode, and non-constant subexpressions.
-  IK_ICEIfUnevaluated,
-  /// This expression is not an ICE, and is not a legal subexpression for one.
-  IK_NotICE
-};
-
 struct ICEDiag {
   ICEKind Kind;
   SourceLocation Loc;
@@ -15533,7 +15522,10 @@
     return NoDiag();
   }
   case Expr::BinaryOperatorClass: {
-    const BinaryOperator *Exp = cast<BinaryOperator>(E);
+    BinaryOperator *Exp = const_cast<BinaryOperator*>(cast<BinaryOperator>(E));
+    if (auto Result = Exp->getICEKind())
+      return ICEDiag(*Result, E->getBeginLoc());
+
     switch (Exp->getOpcode()) {
     case BO_PtrMemD:
     case BO_PtrMemI:
@@ -15551,6 +15543,7 @@
       // C99 6.6/3 allows assignments within unevaluated subexpressions of
       // constant expressions, but they can never be ICEs because an ICE cannot
       // contain an lvalue operand.
+      Exp->setICEKind(IK_NotICE);
       return ICEDiag(IK_NotICE, E->getBeginLoc());
 
     case BO_Mul:
@@ -15579,12 +15572,16 @@
         // we don't evaluate one.
         if (LHSResult.Kind == IK_ICE && RHSResult.Kind == IK_ICE) {
           llvm::APSInt REval = Exp->getRHS()->EvaluateKnownConstInt(Ctx);
-          if (REval == 0)
+          if (REval == 0) {
+            Exp->setICEKind(IK_ICEIfUnevaluated);
             return ICEDiag(IK_ICEIfUnevaluated, E->getBeginLoc());
+          }
           if (REval.isSigned() && REval.isAllOnes()) {
             llvm::APSInt LEval = Exp->getLHS()->EvaluateKnownConstInt(Ctx);
-            if (LEval.isMinSignedValue())
+            if (LEval.isMinSignedValue()) {
+              Exp->setICEKind(IK_ICEIfUnevaluated);
               return ICEDiag(IK_ICEIfUnevaluated, E->getBeginLoc());
+            }
           }
         }
       }
@@ -15592,14 +15589,19 @@
         if (Ctx.getLangOpts().C99) {
           // C99 6.6p3 introduces a strange edge case: comma can be in an ICE
           // if it isn't evaluated.
-          if (LHSResult.Kind == IK_ICE && RHSResult.Kind == IK_ICE)
+          if (LHSResult.Kind == IK_ICE && RHSResult.Kind == IK_ICE) {
+            Exp->setICEKind(IK_ICEIfUnevaluated);
             return ICEDiag(IK_ICEIfUnevaluated, E->getBeginLoc());
+          }
         } else {
           // In both C89 and C++, commas in ICEs are illegal.
+          Exp->setICEKind(IK_NotICE);
           return ICEDiag(IK_NotICE, E->getBeginLoc());
         }
       }
-      return Worst(LHSResult, RHSResult);
+      ICEDiag D = Worst(LHSResult, RHSResult);
+      Exp->setICEKind(D.Kind);
+      return D; 
     }
     case BO_LAnd:
     case BO_LOr: {
@@ -15610,12 +15612,16 @@
         // to actually check the condition to see whether the side
         // with the comma is evaluated.
         if ((Exp->getOpcode() == BO_LAnd) !=
-            (Exp->getLHS()->EvaluateKnownConstInt(Ctx) == 0))
+            (Exp->getLHS()->EvaluateKnownConstInt(Ctx) == 0)) {
+          Exp->setICEKind(RHSResult.Kind);
           return RHSResult;
+        }
+        Exp->setICEKind(RHSResult.Kind);
         return NoDiag();
       }
-
-      return Worst(LHSResult, RHSResult);
+      ICEDiag WorstResult = Worst(LHSResult, RHSResult);
+      Exp->setICEKind(WorstResult.Kind);
+      return WorstResult; 
     }
     }
     llvm_unreachable("invalid binary operator kind");
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -29,6 +29,8 @@
 #include "clang/Basic/TypeTraits.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APSInt.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
@@ -3786,6 +3788,21 @@
   friend class CastExpr;
 };
 
+namespace {
+
+enum ICEKind {
+  /// This expression is an ICE.
+  IK_ICE,
+  /// This expression is not an ICE, but if it isn't evaluated, it's
+  /// a legal subexpression for an ICE. This return value is used to handle
+  /// the comma operator in C99 mode, and non-constant subexpressions.
+  IK_ICEIfUnevaluated,
+  /// This expression is not an ICE, and is not a legal subexpression for one.
+  IK_NotICE,
+};
+
+}
+
 /// A builtin binary operation expression such as "x + y" or "x <= y".
 ///
 /// This expression node kind describes a builtin binary operation,
@@ -3807,10 +3824,18 @@
 class BinaryOperator : public Expr {
   enum { LHS, RHS, END_EXPR };
   Stmt *SubExprs[END_EXPR];
+  llvm::Optional<ICEKind> IK = llvm::None;
 
 public:
   typedef BinaryOperatorKind Opcode;
 
+  llvm::Optional<ICEKind> getICEKind() const { return IK; }
+
+  void setICEKind(ICEKind ik) {
+    assert(!IK && "ICEKind is already set.");
+    IK = ik;
+  }
+
 protected:
   size_t offsetOfTrailingStorage() const;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to