efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land.
In D76096#4540442 <https://reviews.llvm.org/D76096#4540442>, @nickdesaulniers wrote: > Consider the following code: > > long x [] = {1, 2, 3, 4, 5 + 4}; > > Even with some of my recent changes, because the fast path parallel > implementation doesn't currently handle BinaryOperator Exprs, we'll visit all > of the ImplicitCastExpr and IntegerLiteral, then eventually figure out that > we can't constant evaluate the final element (for the wrong reasons; i.e. > VisitBinaryOperator not implemented). That's a slow down because then we'll > fall back to EvaluateAsLValue/EvaluateAsRValue which will then succeed. > > So we pretty much need a fair amount of parallel implementation for the fast > path to succeed. I'm happy to implement all that, if that's the direction > you're advising? > > Or did you mean something else when you said "for structs and arrays?" The given example doesn't fallback in the "bad" way: we don't create an APValue representing `x[]`. If you look at the code for lowering InitListExpr, you'll note that it doesn't actually just recursively Visit() like we do in other places. We do create an lvalue for "5+4", but that isn't expensive in the same way. > Is there another way I can prove the absence of regression to you? At this point, I'm basically convinced the regressions are only going to be in obscure cases we aren't going to find by benchmarking. I'm sure someone will find them eventually, but we can deal with them as they come up, I think. So LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76096/new/ https://reviews.llvm.org/D76096 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
