AaronBallman wrote: > > > Well, this is not going to make a noticeable difference in runtime. > > > https://reviews.llvm.org/D155548 didn't land because there are no > > > measurements to make where this makes a measurable difference. > > > > > > Those changes didn't land because no measurements were attempted. Putting > > up a branch at https://llvm-compile-time-tracker.com/ would help get those > > measurements to at least start to see if there's benefit or harm from the > > changes. > > I did measure it, but only by myself, I can ask Nikita do use the > compile-time tracker as well.
Ah, I didn't realize you had measured on your own. But yeah, having some concrete numbers to play with would help. > > > As for my earlier comment, it would also make sense to rename that > > > function to `computeBitWidth()` or just cache the computed value (we > > > compute it when parsing anyway to diagnose 0 size, etc. right?). > > > > > > Caching the computed value would make sense, but that's sort of the goal of > > D155548, right? That's a generalized caching mechanism that should mean > > when we compute it to diagnose 0 size, we never need to re-compute it again. > > Well yes, we would never compute it again, but we still go through quite a > few function calls to figure out that we've already computed it before. For a > function called `getBitWidthValue()` I basically assume it has exactly one > statement: `return BitWidthValue`. I'm hopeful the optimizer ends up inlining at least some of those calls, but otherwise, yeah you're right. > I thought the bitwidth expression will always be a `ConstantExpr`, unless > it's value-dependent (in which case it will be a `ConstantExpr` when we > instantiate it, right?), but I might be wrong. Per spec, it needs to be a constant expression. In terms of our implementation, I would also assume it would either be `ConstantExpr` or `IntegerLiteral` (which, strangely is an `Expr` and not a `ConstantExpr`) https://github.com/llvm/llvm-project/pull/66203 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
