GH-41020: [C++] Introduce portable compiler assumptions

[felipecrv]

Rationale for this change

Allow portable use of the "assumption" feature of modern GCC, Clang, and MSVC compilers.

What changes are included in this PR?

• Documenting of macros in macros.h
• Addition of ARROW_COMPILER_ASSUME
• One simple change where this macro simplifies the existing code while keeping the common case of a loop branch dependent only on local variables

Are these changes tested?

By existing tests.

• GitHub Issue: [C++] Introduce portable compiler assumptions #41020

[github-actions]

⚠️ GitHub issue #41020 has been automatically assigned in GitHub to PR creator.

[mapleFU]

I wonder would llvm / gcc have a pass or other things finding out that could it find the assertions? Seems that the rule in this case is trivial, maybe we can use this when it's not so trivial and have some benchmark shows that performance would benifits from this?

[felipecrv]

I wonder would llvm / gcc have a pass or other things finding out that could it find the assertions? Seems that the rule in this case is trivial, maybe we can use this when it's not so trivial and have some benchmark shows that performance would benifits from this?

Yes, they already do a great job finding assumptions [1] and the C++ standard allows many assumptions to be made. e.g. every pointer dereference assumes that the pointer is not-null, divisors are assumed to be non-0, signed arithmetic is assumed to not overflow, etc.

[1] assertions and assumptions are very different things -> https://blog.regehr.org/archives/1096

[mapleFU]

Thanks for introducing this. IMO assume is something like restrict. And it might not only brings optimization[1]. I'm glad to have this, but when we change the code maybe we need benchmark data

[1] https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609

[pitrou]

And it might not only brings optimization[1]. I'm glad to have this, but when we change the code maybe we need benchmark data

[1] https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609

Hmm, interesting. Thank you for the pointer. Also, assume might then have different drawbacks depending on the compiler, which is rather annoying.

[felipecrv]

I removed the use of the macro and force-pushed.

[mapleFU]

+1

[felipecrv]

FYI: I messed up a bit on the last force push and didn't include the alphabetizing macros commit. Nothing else was lost though.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit aeb1618.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 12 possible false positives for unstable benchmarks that are known to sometimes produce them.