Cleanup useless/non-constexpr branchs #2025
Conversation
|
I am OK with the x86 fixes. |
|
I am against replaicing constexpr with consteval:
|
| @@ -65,52 +65,52 @@ struct swap_adjacent_t | |||
| } | |||
|
|
|||
| template<simd_value T, std::ptrdiff_t G> | |||
| static constexpr std::ptrdiff_t level(eve::as<T> tgt, eve::fixed<G> g) | |||
There was a problem hiding this comment.
these tests in level were deliberately not if constexpr where posssible.
main consideration is that if constexpr is probably slower to compile.
it also, for example, does not to do else branches.
| { | ||
| using half_t = decltype(T {}.slice(lower_)); | ||
| return level(as<half_t> {}, g); | ||
| } |
There was a problem hiding this comment.
I'm not sure thids change improves things. maybe to not compile level(half)
There was a problem hiding this comment.
Yes, that was the idea.
| if constexpr (current_api == avx) return 4; | ||
| if constexpr (g_size >= 8) return 2; | ||
| if constexpr (g_size >= 4) return 3; | ||
| if constexpr (g_size >= 2 && current_api == avx512) return 3; |
There was a problem hiding this comment.
you see - now you push things to comile time switching instead of just evaluating. I don't know how this works under the hood, but it seems worse.
There was a problem hiding this comment.
Could you expand on that a little bit ? I don't think I quite get it.
include/eve/arch/cpu/top_bits.hpp
Outdated
| { | ||
| if constexpr ( is_aggregated ) return top_bits<half_logical>::bits_per_element; | ||
| else return decltype(movemask(logical_type{}).second){}(); | ||
| } | ||
|
|
||
| static constexpr bool is_cheap_impl() | ||
| static consteval bool is_cheap_impl() |
There was a problem hiding this comment.
same story here - we don't want the ifs to be constexpr when t doesn't have to. And I don't like consteval for no reason.
|
Setting aside the consteval aspect (which I will revert as some of these functions are indeed used at runtime), a good part of these branchs were already if-constexpr. Also, these branchs depends on template parameters, wouldn't marking them as constexpr help with the compile times, if only a little ? |
Where I had to.
this is my thinking, which may be wrong. I do not know for sure. I work with some people I can ask - so I will when I have the chance. When you do an I suspect that if constexpr mechanism is similar to some template stuff and might be heavier on the compiler. We can park this until I confirm. |
|
My reasoning is as follows: since these branches can be marked as constexpr, the compiler will likely eliminate them at compile time. If that’s the case, marking them as constexpr might reduce the scope of this optimization for each function template instantiation. I would also like to know which approcach is in fact more compiler-friendly. |
everything here is compile time! We want less compile time. |
|
I have extracted the constexpr-related stuff into another draft PR, only the "bug fix" remains. |
No description provided.