aaron.ballman accepted this revision. aaron.ballman added a comment. In D155548#4653363 <https://reviews.llvm.org/D155548#4653363>, @cor3ntin wrote:
> @aaron.ballman Given the recent discussion we had on both github > (https://github.com/llvm/llvm-project/pull/66203) and privately, I'm going to > accept that. > We only create ConstantExpr for ICE and immediate invocations, so it would > have a much less limited and impact that first envisioned (we should fix > that, cf https://github.com/llvm/llvm-project/issues/61425), and so we don't > have good benchmarks were this patch makes things better, > but it also doesn't make things worse and in helps in limited cases. > > We should probably investigate as a follow up whether we should use > `ConstantExpr` in more places I'm very skeptical of performance improvements where we can't actually measure that performance was improved. However, I actually agree with @cor3ntin in this case -- we're not using `ConstantExpr` in places where I would have expected to see it used -- instead we do a dance like we do here with `IntegerLiteral` and `CXXBoolLiteralExpr`, so this would currently require `consteval`-heavy code to see what kind of performance improvements there are, and there's not a lot of extant code for us to test against. I'm accepting with some reluctance given the lack of demonstration that this is an improvement; please keep an eye out for new issues filed against Clang 18 for performance regressions. I don't expect this to cause any issues, but best to keep it in mind given the testing done showing increases in wall clock time as well as decreases. Hopefully follow-up work will improve caching by using `ConstantExpr` in more places. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155548/new/ https://reviews.llvm.org/D155548 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
