Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use if consteval after compiler support arrives #5225

Open
StephanTLavavej opened this issue Jan 7, 2025 · 5 comments
Open

Use if consteval after compiler support arrives #5225

StephanTLavavej opened this issue Jan 7, 2025 · 5 comments
Labels
enhancement Something can be improved

Comments

@StephanTLavavej
Copy link
Member

https://en.cppreference.com/w/cpp/23 indicates that Clang 14 began supporting C++23 WG21-P1938R3 if consteval, and it's available as Future Technology in C++20 mode: https://godbolt.org/z/MhjoaxW6a

For C1XX, MSVC-PR-598052 was merged on 2024-12-12 and will ship in VS 2022 17.14 Preview 1. It was implemented as Future Technology that we can access with a suppressible warning (C5282). Presumably this also goes back to C++20 but not earlier (haven't checked) as consteval became a keyword in C++20.

For EDG in VS 2022 17.13 Preview 2, if consteval appears to be supported in C++23 mode, but C++20 mode emits a syntax error. (As EDG isn't a codegen compiler for us, this is the least important.)

I haven't checked CUDA.

While I'm not looking forward to macroization, we should look into using if consteval after C1XX support ships, because it can significantly improve debug codegen, and should give the release optimizer slightly less work to do.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Jan 7, 2025
@CaseyCarter
Copy link
Member

CaseyCarter commented Jan 8, 2025

This would be particularly nice for our hand-vectorized algorithms, which currently produce enormous debug codegen. When not optimizing the compiler doesn't realize that std::is_constant_evaluated()) is a constant expression, so it emits code for both the vectorized and non-vectorized paths. We've gotten some nasty glares from the compiler backend folk who monitor codegen size because of this.

I suppose we can replace the pattern:

if constexpr (/* the algorithm can be hand-vectorized for the pertinent types */) {
    if (!std::is_constant_evaluated()) {
        /* vectorized implementation */
    }
}

/* vanilla implementation */

with

if consteval {
    vanilla_implementation();
} else if constexpr (/* the algorithm can be hand-vectorized for the pertinent types */) {
    /* vectorized implementation */
} else {
    vanilla_implementation();
}

which requires the vanilla (non-vectorized) implementation to be extracted into a separate function (perhaps a lambda) to avoid duplication. This should reduce debug codegen since if consteval and if constexpr are both compile-time branches.

(Shout if you see a way to perform the dispatch that doesn't require repetition and/or extraction - I don't.)

@DeadlyRedCube
Copy link

DeadlyRedCube commented Jan 12, 2025

I went to check this on godbolt and forgot that MSVC doesn't quite have if constexpr yet, but would this fit the bill?

if constexpr (/* the algorithm can be hand-vectorized for the pertinent types */) {
    if consteval {
        /* do nothing */
    } else {
        /* vectorized implementation and a return */
    }
}

/* vanilla implementation */

or would that still have the same behavior?

@libbooze
Copy link

libbooze commented Jan 12, 2025

(Shout if you see a way to perform the dispatch that doesn't require repetition and/or extraction - I don't.)

I think this is the cleanest way, if you have time ptal.
While it is still 2 functions I still find it relatively clean since dispatch and logic are split and vanilla and simd logic can live in same function without repetition.
You might get a some more nasty compiler dev glares because it involves more templates so it may slow down compile...

https://godbolt.org/z/s3rbo8vaa

@StephanTLavavej
Copy link
Member Author

@DeadlyRedCube The "fall through" results in codegen being emitted for both vectorized and vanilla, since the FE doesn't perform any sort of dead code elimination and the BE under /Od doesn't.

@DeadlyRedCube
Copy link

@StephanTLavavej Ah, darn. Was hoping since it was a clear compile time branch that it could do it, but that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

No branches or pull requests

4 participants