On Mon, Feb 12, 2024 at 01:31:30PM -0800, Andres Freund wrote: > One thing that's worth checking is if this ends up with *worse* code when the > comparators are inlined. I think none of the changed comparators will end up > getting used with an inlined sort, but ...
Yeah, AFAICT the only inlined sorts are in tuplesort.c and bufmgr.c, and the patches don't touch those files. > The reason we could end up with worse code is that when inlining the > comparisons would make less sense for the compiler. Consider e.g. > return DO_COMPARE(a, b) < 0 ? > (DO_COMPARE(b, c) < 0 ? b : (DO_COMPARE(a, c) < 0 ? c : a)) > : (DO_COMPARE(b, c) > 0 ? b : (DO_COMPARE(a, c) < 0 ? a : c)); > > With a naive implementation the compiler will understand it only cares about > a < b, not about the other possibilities. I'm not sure that's still true with > the more complicated optimized version. You aren't kidding [0]. Besides perhaps adding a comment in sort_template.h, is there anything else you think we should do about this now? [0] https://godbolt.org/z/bbTqK54zK -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
