Re: [eigen] Re: Strong inlining is sometimes ignored...

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

2009/10/15 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
> It is easily explained.
>
> When SSE is disabled,
>
> Redux.h line 92: ei_redux_novec_unroller<...>::run(...)
>
> is called. This implementation simply unrolls with the functor.
>
> In case SSE is enabled,
>
> Redux.h line 299: ei_redux_novec_unroller<..., LinearVectorization,
> CompleteUnrolling>::run(...)
>
> is called. This code contains an if-clause for the case when
> 'VectorizationSize' is not equal to 'Size' which calls the functor for the
> remaining vector entries. In case, we have even sized vectors, this code is
> compiled with zero length.

Thanks for the explanation, that makes perfect sense.

>
> The easiest way to get really rid of this if-clause is to write another
> specialized functor for this case - this is possible since it can be
> computed at compile time if the expression is fully vectorizable or not.
> Specializing the whole ei_redux_impl might be a bit painful since it would
> require to add an additional boolean flag - for all cases.

I don't understand: why is it not enough to add the specialization for
Length=0 ?

A dead if doesn't cost anything by itself, it goes away at
compile-time. It can be useful to replace by template-based selection
as you suggest when that allows to avoid the instantiation of heavier
templates (thus it's an optimization of compilation time). But here,
since you've added the Length=0 specialization, all you're avoiding is
the instantiation of that specialization which is very small, actually
it's lighter than the template selector that you are introducing, so I
am not sure that it's worth it.

Or am I missing something here?

>
> I just created a patch with all fixes...

So, I fully agree about the removal of the assert and the introduction
of the Length=0 specialization; I still need to be convinced about the
introduction of the template selector.

I noticed your nice comment for the Length=0 specialization, it's
really good that you wrote it because otherwise that's the kind of
thing that could be mistakenly removed at a later date.

>
> On Thu, Oct 15, 2009 at 3:12 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
> wrote:
>>
>> Second, I'd like to understand, why is this in
>> ei_redux_novec_unroller, while precisely you found that the issue
>> happened with vectorizable cases, so why not in ei_redux_vec_unroller
>> ?
>
> No idea... I have to take a look at that. It seems that when
> EIGEN_DONT_VECTORIZE is enabled LinearVectorization is never used. So
> _novec_ still allows linear vectorization. I have to double check the code.

Yes but actually, your answer to my 1st question seems to also answer
that one: the novec_unroller is called for the "last few scalars" in
the vectorized path, with length=0 in the fully-vectorizable case,
that's a sufficient explanation for why we get those warnings in
ei_redux_novec_unroller when enabling vectorization!

Thanks,
Benoit