[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-08 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 168712. Repository: rC Clang https://reviews.llvm.org/D52854 Files: include/clang/AST/Expr.h include/clang/AST/Stmt.h include/clang/Sema/Sema.h lib/AST/Expr.cpp lib/AST/ExprConstant.cpp lib/CodeGen/CGBuiltin.cpp lib/Parse/ParseDecl.cpp

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-08 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 168708. void added a comment. I updated the patch that implements putting the "can delay evaluation" bit into the CallExprBitfields. PTAL. Repository: rC Clang https://reviews.llvm.org/D52854 Files: include/clang/AST/Expr.h include/clang/AST/Stmt.h

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-08 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. Here's a WIP patch to show what I'm talking about.F7375980: example.diff Repository: rC Clang https://reviews.llvm.org/D52854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-08 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. No, I understood that. But the issue is that you then need to specify this new expression class in over 23 different files: various macros, switch statements, etc. Repository: rC Clang https://reviews.llvm.org/D52854 ___

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I don't think that was the approach @rsmith was suggesting; you don't need to wrap every possible kind of expression. Rather, at the point where the expression is required to be constant, add a single expression which wraps the entire expression tree to indicate

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-07 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. @rsmith I've been implementing your suggestion, but it's quickly becoming ridiculous. Just to implement a `ConstExpr` wrapper class, I need to touch ~23 files, virtually all changes being boilerplate code to forward the action to the wrapped expression. And this is before

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 168395. void marked an inline comment as done. Repository: rC Clang https://reviews.llvm.org/D52854 Files: include/clang/AST/Expr.h include/clang/Sema/Sema.h lib/AST/Expr.cpp lib/AST/ExprConstant.cpp lib/CodeGen/CGBuiltin.cpp

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Bill Wendling via Phabricator via cfe-commits
void marked 9 inline comments as done. void added a comment. In https://reviews.llvm.org/D52854#1255755, @rsmith wrote: > Essentially, you would add a `ConstExpr` node (or similar), teach > `IgnoreImplicit` and friends to step over it I've been trying to do this to no avail. Here's the code I

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: test/Sema/builtins.c:125 +// __builtin_constant_p cannot resolve non-constants as a file scoped array. +int expr; such as Otherwise it's possible to read this as the non-constants being resolved to a file

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); nickdesaulniers wrote: > void wrote: > >

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Rather than adding a `CanDelayEvaluation` flag to call expressions, I think it would be substantially better to introduce a new `Expr` wrapper node for expressions that are required to be constant expressions. (That'd eventually also give us a place to cache the

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); void wrote: > nickdesaulniers wrote: > >

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Bill Wendling via Phabricator via cfe-commits
void added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); nickdesaulniers wrote: > nickdesaulniers wrote: > >

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); nickdesaulniers wrote: > `std::for_each`? Sorry, posted

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); `std::for_each`? https://reviews.llvm.org/D52854

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 168356. void added a comment. Removed unused field. https://reviews.llvm.org/D52854 Files: include/clang/AST/Expr.h include/clang/Sema/Sema.h lib/AST/Expr.cpp lib/AST/ExprConstant.cpp lib/CodeGen/CGBuiltin.cpp lib/Sema/SemaExpr.cpp

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-04 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15604 +public EvaluatedExprVisitor { + Sema + ../tools/clang/lib/Sema/SemaExpr.cpp:15619:13: warning: private field 'S' is not used [-Wunused-private-field] Sema

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-03 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. This is related to https://reviews.llvm.org/D4276, which I believe has been deleted. However, I believe that a modified version of that patch will work. The idea of this patch is *not* to be fully compatible with gcc's implementation, but to expand clang's implementation

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

2018-10-03 Thread Bill Wendling via Phabricator via cfe-commits
void created this revision. void added reviewers: jyknight, eli.friedman, resistor, janusz. Herald added subscribers: cfe-commits, kristina. A __builtin_constant_p may end up with a constant after inlining. Use the is.constant intrinsic if we're unable to determine if it's a constant. See: