[llvm] [clang] [lld] [libc] [mlir] [libcxx] [clang-tools-extra] [flang] [compiler-rt] [libc++][hardening] Don't trigger redundant checks in the fast mode. (PR #77176)

Louis Dionne via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 22 12:26:37 PST 2024


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


More information about the cfe-commits mailing list