https://github.com/ldionne approved this pull request.

@philnik777 
> Does this actually improve the performance anywhere? AFAICT all we gain is 
> avoiding the messages being generated in the binary, but I'm not convinced 
> that's much of a problem. e.g. in the `std::next` case, there doesn't seem to 
> be any overhead as-is with any optimizations enabled: 
> https://godbolt.org/z/768E1YMsf

I think that's a great question. I think it will depend on the circumstances, 
and I definitely expect that in some cases the compiler won't be able to do as 
good a job as we'd like with de-duplicating assertions. The `std::next` example 
you showed is not very challenging to de-duplicate, but other checks could be 
more challenging depending on inlining behavior, for example.

Apart from the question of "does this make the code faster", I think this macro 
does provide semantic value because it gives us a tool to say "this assertion 
exists, but you can rely on another one to have the same effect at the end of 
the day". I suspect this will resolve a lot of discussions like "should I add 
an assertion here? it's technically already checked elsewhere so we could avoid 
checking twice". If we have this tool, this becomes a non-issue. Our policy can 
be to simply add assertions when they make sense from the semantic point of 
view, and use this macro to ensure that codegen is still optimal.

I'd like this to land in LLVM 18. @philnik777 if you think this necessitates 
more discussion, please let us know and we can set up some time to address it 
in LLVM 19. I'd even be OK with reverting in LLVM 19 in case we find out that 
this is definitely not the way to go, but barring new information this seems 
like a good direction to me.

https://github.com/llvm/llvm-project/pull/77176
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to