tbaederr 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.

> 
> > 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 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.



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

Reply via email to