[PATCH] D131416: [Clang][BinaryOperator] cache ICEKind
justinstitt created this revision.
Herald added a project: All.
justinstitt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Signed-off-by: Justin Stitt
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131416
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
@@ -15283,27 +15283,6 @@
// in the rare cases where CheckICE actually cares about the evaluated
// value, it calls into Evaluate.
-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;
-
- ICEDiag(ICEKind IK, SourceLocation l) : Kind(IK), Loc(l) {}
-};
-
-}
static ICEDiag NoDiag() { return ICEDiag(IK_ICE, SourceLocation()); }
@@ -15322,7 +15301,7 @@
return NoDiag();
}
-static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
+static ICEDiag CheckICE(Expr* E, const ASTContext &Ctx) {
assert(!E->isValueDependent() && "Should not see value dependent exprs!");
if (!E->getType()->isIntegralOrEnumerationType())
return ICEDiag(IK_NotICE, E->getBeginLoc());
@@ -15533,7 +15512,9 @@
return NoDiag();
}
case Expr::BinaryOperatorClass: {
-const BinaryOperator *Exp = cast(E);
+BinaryOperator *Exp = cast(E);
+if (Exp->IK.has_value()) return ICEDiag(Exp->IK.value(), E->getBeginLoc());
+/* if (Exp->IK != IK_NotYetEvaluated) return ICEDiag(Exp->IK, E->getBeginLoc()); */
switch (Exp->getOpcode()) {
case BO_PtrMemD:
case BO_PtrMemI:
@@ -15551,6 +15532,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->IK = IK_NotICE;
return ICEDiag(IK_NotICE, E->getBeginLoc());
case BO_Mul:
@@ -15579,12 +15561,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->IK = 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->IK = IK_ICEIfUnevaluated;
return ICEDiag(IK_ICEIfUnevaluated, E->getBeginLoc());
+}
}
}
}
@@ -15592,14 +15578,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->IK = IK_ICEIfUnevaluated;
return ICEDiag(IK_ICEIfUnevaluated, E->getBeginLoc());
+ }
} else {
// In both C89 and C++, commas in ICEs are illegal.
+ Exp->IK = IK_NotICE;
return ICEDiag(IK_NotICE, E->getBeginLoc());
}
}
- return Worst(LHSResult, RHSResult);
+ ICEDiag D = Worst(LHSResult, RHSResult);
+ Exp->IK = D.Kind;
+ return D;
}
case BO_LAnd:
case BO_LOr: {
@@ -15610,12 +15601,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->IK = RHSResult.Kind;
return RHSResult;
+}
+Exp->IK = IK_ICE;
return NoDiag();
}
-
- return Worst(LHSResult, RHSResult);
+ ICEDiag WorstResult = Worst(LHSResult, RHSResult);
+ Exp->IK = WorstResult.Kind;
+ return WorstResult;
}
}
llvm_unreachable("invalid binary operator kind");
@@ -15747,7 +15742,6 @@
if (Ctx.getLangOpts().CPlusPlus11)
return EvaluateCPlusPlus11IntegralConstantExpr(Ctx, this, nullptr, Loc);
-
ICEDiag D = CheckICE(this, Ctx);
if (D.Kind != IK_ICE) {
if (Loc) *Loc = D.Loc;
Index: clang/include/clang/AST/Expr.h
===
--- cla
[PATCH] D131416: [Clang][BinaryOperator] cache ICEKind
justinstitt updated this revision to Diff 450872.
justinstitt edited the summary of this revision.
justinstitt added a reviewer: nickdesaulniers.
justinstitt added a comment.
- test adding reviewers
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131416/new/
https://reviews.llvm.org/D131416
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
@@ -15283,27 +15283,6 @@
// in the rare cases where CheckICE actually cares about the evaluated
// value, it calls into Evaluate.
-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;
-
- ICEDiag(ICEKind IK, SourceLocation l) : Kind(IK), Loc(l) {}
-};
-
-}
static ICEDiag NoDiag() { return ICEDiag(IK_ICE, SourceLocation()); }
@@ -15322,7 +15301,7 @@
return NoDiag();
}
-static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
+static ICEDiag CheckICE(Expr* E, const ASTContext &Ctx) {
assert(!E->isValueDependent() && "Should not see value dependent exprs!");
if (!E->getType()->isIntegralOrEnumerationType())
return ICEDiag(IK_NotICE, E->getBeginLoc());
@@ -15533,7 +15512,9 @@
return NoDiag();
}
case Expr::BinaryOperatorClass: {
-const BinaryOperator *Exp = cast(E);
+BinaryOperator *Exp = cast(E);
+if (Exp->IK.has_value()) return ICEDiag(Exp->IK.value(), E->getBeginLoc());
+/* if (Exp->IK != IK_NotYetEvaluated) return ICEDiag(Exp->IK, E->getBeginLoc()); */
switch (Exp->getOpcode()) {
case BO_PtrMemD:
case BO_PtrMemI:
@@ -15551,6 +15532,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->IK = IK_NotICE;
return ICEDiag(IK_NotICE, E->getBeginLoc());
case BO_Mul:
@@ -15579,12 +15561,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->IK = 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->IK = IK_ICEIfUnevaluated;
return ICEDiag(IK_ICEIfUnevaluated, E->getBeginLoc());
+}
}
}
}
@@ -15592,14 +15578,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->IK = IK_ICEIfUnevaluated;
return ICEDiag(IK_ICEIfUnevaluated, E->getBeginLoc());
+ }
} else {
// In both C89 and C++, commas in ICEs are illegal.
+ Exp->IK = IK_NotICE;
return ICEDiag(IK_NotICE, E->getBeginLoc());
}
}
- return Worst(LHSResult, RHSResult);
+ ICEDiag D = Worst(LHSResult, RHSResult);
+ Exp->IK = D.Kind;
+ return D;
}
case BO_LAnd:
case BO_LOr: {
@@ -15610,12 +15601,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->IK = RHSResult.Kind;
return RHSResult;
+}
+Exp->IK = IK_ICE;
return NoDiag();
}
-
- return Worst(LHSResult, RHSResult);
+ ICEDiag WorstResult = Worst(LHSResult, RHSResult);
+ Exp->IK = WorstResult.Kind;
+ return WorstResult;
}
}
llvm_unreachable("invalid binary operator kind");
@@ -15747,7 +15742,6 @@
if (Ctx.getLangOpts().CPlusPlus11)
return EvaluateCPlusPlus11IntegralConstantExpr(Ctx, this, nullptr, Loc);
-
ICEDiag D = CheckICE(this, Ctx);
if (D.Kind != IK_ICE) {
if (Loc) *Loc = D.Loc;
Index: clang/include/clang/AST/Expr.h
[PATCH] D131416: [Clang][BinaryOperator] cache ICEKind
justinstitt updated this revision to Diff 450961.
justinstitt marked 7 inline comments as done.
justinstitt added a comment.
create getter/setter for optional ICEKind
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131416/new/
https://reviews.llvm.org/D131416
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(E);
+BinaryOperator *Exp = const_cast(cast(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,
+};
+
+}
[PATCH] D131447: [Clang][BinaryOperator] memoize ICEKind for BinaryOperator Exprs
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(E);
+BinaryOperator *Exp = const_cast(cast(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 {
[PATCH] D131416: [Clang][BinaryOperator] cache ICEKind
justinstitt updated this revision to Diff 450964.
justinstitt added a comment.
update commit message
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131416/new/
https://reviews.llvm.org/D131416
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(E);
+BinaryOperator *Exp = const_cast(cast(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
[PATCH] D131416: [Clang][BinaryOperator] memoize ICEKind for BinaryOperator Exprs
justinstitt updated this revision to Diff 450966.
justinstitt retitled this revision from "[Clang][BinaryOperator] cache ICEKind"
to "[Clang][BinaryOperator] memoize ICEKind for BinaryOperator Exprs".
justinstitt edited the summary of this revision.
justinstitt added a comment.
Herald added a subscriber: pengfei.
add profile data to commit message
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131416/new/
https://reviews.llvm.org/D131416
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(E);
+BinaryOperator *Exp = const_cast(cast(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 comm
[PATCH] D131532: [Sema] Only invoke DiagnoseNullConversions for CPlusPlus (-Wnull-conversion)
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. -Wnull-conversion is not supported in C and therefore DiagnoseNullConversions should only be called for C++. When testing this change on Linux Kernel builds (x86_64 defconfig) we see an approximate 2.1% reduction in build times! This is mainly caused by two methods no longer needing to be invoked as often: 1. ExprConstant::CheckICE and 2) IntExprEvaluator::VisitBinaryOperator. The former has had its total cpu cycles reduced by ~90% and the latter by about ~50% for Kernel builds. The C++ Standard (https://isocpp.org/files/papers/N4860.pdf) states that "NULL is an implementation-defined null pointer constant" and "possible definitions include 0, 0L, but not (void*)0". Which may explain the difference in C and C++ as seen here: https://godbolt.org/z/c5j8fY39 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D131532 Files: clang/lib/Sema/SemaChecking.cpp Index: clang/lib/Sema/SemaChecking.cpp === --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/SemaChecking.cpp @@ -13865,7 +13865,8 @@ } } - DiagnoseNullConversion(S, E, T, CC); + if (S.getLangOpts().CPlusPlus) +DiagnoseNullConversion(S, E, T, CC); S.DiscardMisalignedMemberAddress(Target, E); Index: clang/lib/Sema/SemaChecking.cpp === --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/SemaChecking.cpp @@ -13865,7 +13865,8 @@ } } - DiagnoseNullConversion(S, E, T, CC); + if (S.getLangOpts().CPlusPlus) +DiagnoseNullConversion(S, E, T, CC); S.DiscardMisalignedMemberAddress(Target, E); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131532: [Sema] Avoid isNullPointerConstant invocation
justinstitt updated this revision to Diff 451687.
justinstitt added a comment.
use @rtrieu 's suggestion regarding not invoking isNullPointerConstant as
opposed to checking for C++ lang opt.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131532/new/
https://reviews.llvm.org/D131532
Files:
clang/lib/Sema/SemaChecking.cpp
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,10 +13339,10 @@
return;
// Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
- const Expr::NullPointerConstantKind NullKind =
- E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
- if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
-return;
+ const Expr *NewE = E->IgnoreParenImpCasts();
+ bool GNUNull = isa(NewE);
+ bool NullPtr = NewE->getType()->isNullPtrType();
+ if (!GNUNull && !NullPtr) return;
// Return if target type is a safe conversion.
if (T->isAnyPointerType() || T->isBlockPointerType() ||
@@ -13358,7 +13358,7 @@
CC = S.SourceMgr.getTopMacroCallerLoc(CC);
// __null is usually wrapped in a macro. Go up a macro if that is the case.
- if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+ if (GNUNull && Loc.isMacroID()) {
StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
Loc, S.SourceMgr, S.getLangOpts());
if (MacroName == "NULL")
@@ -13370,7 +13370,7 @@
return;
S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
- << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+ << NullPtr << T << SourceRange(CC)
<< FixItHint::CreateReplacement(Loc,
S.getFixItZeroLiteralForType(T, Loc));
}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,10 +13339,10 @@
return;
// Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
- const Expr::NullPointerConstantKind NullKind =
- E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
- if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
-return;
+ const Expr *NewE = E->IgnoreParenImpCasts();
+ bool GNUNull = isa(NewE);
+ bool NullPtr = NewE->getType()->isNullPtrType();
+ if (!GNUNull && !NullPtr) return;
// Return if target type is a safe conversion.
if (T->isAnyPointerType() || T->isBlockPointerType() ||
@@ -13358,7 +13358,7 @@
CC = S.SourceMgr.getTopMacroCallerLoc(CC);
// __null is usually wrapped in a macro. Go up a macro if that is the case.
- if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+ if (GNUNull && Loc.isMacroID()) {
StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
Loc, S.SourceMgr, S.getLangOpts());
if (MacroName == "NULL")
@@ -13370,7 +13370,7 @@
return;
S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
- << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+ << NullPtr << T << SourceRange(CC)
<< FixItHint::CreateReplacement(Loc,
S.getFixItZeroLiteralForType(T, Loc));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131532: [Sema] Avoid isNullPointerConstant invocation
justinstitt updated this revision to Diff 451902.
justinstitt edited the summary of this revision.
justinstitt added a comment.
add more descriptive names, add more detail to revision summary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131532/new/
https://reviews.llvm.org/D131532
Files:
clang/lib/Sema/SemaChecking.cpp
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,10 +13339,10 @@
return;
// Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
- const Expr::NullPointerConstantKind NullKind =
- E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
- if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
-return;
+ const Expr *NewE = E->IgnoreParenImpCasts();
+ bool IsGNUNullExpr = isa(NewE);
+ bool HasNullPtrType = NewE->getType()->isNullPtrType();
+ if (!IsGNUNullExpr && !HasNullPtrType) return;
// Return if target type is a safe conversion.
if (T->isAnyPointerType() || T->isBlockPointerType() ||
@@ -13358,7 +13358,7 @@
CC = S.SourceMgr.getTopMacroCallerLoc(CC);
// __null is usually wrapped in a macro. Go up a macro if that is the case.
- if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+ if (IsGNUNullExpr && Loc.isMacroID()) {
StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
Loc, S.SourceMgr, S.getLangOpts());
if (MacroName == "NULL")
@@ -13370,7 +13370,7 @@
return;
S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
- << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+ << HasNullPtrType << T << SourceRange(CC)
<< FixItHint::CreateReplacement(Loc,
S.getFixItZeroLiteralForType(T, Loc));
}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,10 +13339,10 @@
return;
// Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
- const Expr::NullPointerConstantKind NullKind =
- E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
- if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
-return;
+ const Expr *NewE = E->IgnoreParenImpCasts();
+ bool IsGNUNullExpr = isa(NewE);
+ bool HasNullPtrType = NewE->getType()->isNullPtrType();
+ if (!IsGNUNullExpr && !HasNullPtrType) return;
// Return if target type is a safe conversion.
if (T->isAnyPointerType() || T->isBlockPointerType() ||
@@ -13358,7 +13358,7 @@
CC = S.SourceMgr.getTopMacroCallerLoc(CC);
// __null is usually wrapped in a macro. Go up a macro if that is the case.
- if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+ if (IsGNUNullExpr && Loc.isMacroID()) {
StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
Loc, S.SourceMgr, S.getLangOpts());
if (MacroName == "NULL")
@@ -13370,7 +13370,7 @@
return;
S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
- << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+ << HasNullPtrType << T << SourceRange(CC)
<< FixItHint::CreateReplacement(Loc,
S.getFixItZeroLiteralForType(T, Loc));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131532: [Sema] Avoid isNullPointerConstant invocation
justinstitt updated this revision to Diff 451955.
justinstitt added a comment.
move return to new line, as per clang-format.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131532/new/
https://reviews.llvm.org/D131532
Files:
clang/lib/Sema/SemaChecking.cpp
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,9 +13339,10 @@
return;
// Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
- const Expr::NullPointerConstantKind NullKind =
- E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
- if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
+ const Expr *NewE = E->IgnoreParenImpCasts();
+ bool IsGNUNullExpr = isa(NewE);
+ bool HasNullPtrType = NewE->getType()->isNullPtrType();
+ if (!IsGNUNullExpr && !HasNullPtrType)
return;
// Return if target type is a safe conversion.
@@ -13358,7 +13359,7 @@
CC = S.SourceMgr.getTopMacroCallerLoc(CC);
// __null is usually wrapped in a macro. Go up a macro if that is the case.
- if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+ if (IsGNUNullExpr && Loc.isMacroID()) {
StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
Loc, S.SourceMgr, S.getLangOpts());
if (MacroName == "NULL")
@@ -13370,7 +13371,7 @@
return;
S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
- << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+ << HasNullPtrType << T << SourceRange(CC)
<< FixItHint::CreateReplacement(Loc,
S.getFixItZeroLiteralForType(T, Loc));
}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,9 +13339,10 @@
return;
// Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
- const Expr::NullPointerConstantKind NullKind =
- E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
- if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
+ const Expr *NewE = E->IgnoreParenImpCasts();
+ bool IsGNUNullExpr = isa(NewE);
+ bool HasNullPtrType = NewE->getType()->isNullPtrType();
+ if (!IsGNUNullExpr && !HasNullPtrType)
return;
// Return if target type is a safe conversion.
@@ -13358,7 +13359,7 @@
CC = S.SourceMgr.getTopMacroCallerLoc(CC);
// __null is usually wrapped in a macro. Go up a macro if that is the case.
- if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+ if (IsGNUNullExpr && Loc.isMacroID()) {
StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
Loc, S.SourceMgr, S.getLangOpts());
if (MacroName == "NULL")
@@ -13370,7 +13371,7 @@
return;
S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
- << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+ << HasNullPtrType << T << SourceRange(CC)
<< FixItHint::CreateReplacement(Loc,
S.getFixItZeroLiteralForType(T, Loc));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
