Re: [eigen] eigen short support: discussion thread

[Rohit Garg <rpg.314@xxxxxxxxx>]

On Sat, Aug 29, 2009 at 10:42 PM, Benoit Jacob<jacob.benoit.1@xxxxxxxxx> wrote:
> 2009/8/29 Rohit Garg <rpg.314@xxxxxxxxx>:
>> 1) With regard to the __m128i being the common sse type for short
>> vectors and int vectors, how about we let just the Packet8s be a
>> typedef for a struct/class containing a __m128i. This way, they'll be
>> different types.
>>
>> I want to ask if people here think this would work and if it will be
>> cheaper to implement than other ways?
>
> Actually, you raise a very good point. The solution that I proposed
> looks like this except that I only proposed to make such a struct wrap
> the packet type, not a packet object. I proposed an empty struct. Now
> it's tru that when we want to implement e.g. ei_padd, it looks like
>
> packet_type ei_padd(const packet_type&, const packet_type&)
>
> and so if we want " z = ei_padd(x,y) " to work, we need packet_type to
> actually contain an object, not just a typedef. My solution would have
> forced one to pass a "Scalar" template parameter to all the ei_p...
> methods taking packets -- since these packet _objects_ still didn't
> remember about the scalar type.
>
> So, +1 to your idea. Gael?

I assume that this means we can just copy-paste int-to-short routines
(for the most part) then. :)  I am interested in avoiding changes
outside PacketMath.h where ever possible.

> Your struct could be called ei_packet. It remains to be seen if
> ei_packet_traits can be merged into it, or not.
>
>> 2) What extra needs to be done for the basic short support to be
>> merged into eigen mainline? There's a unit test (a simple one) in my
>> branch.
>>
>> http://bitbucket.org/rpg/eigen2-rpg-branch/src/tip/test/short.cpp
>>
>> Is it enough for the code to be commited? If not, what else is
>> missing? (vectorization will obviously take a bit more time)
>
> First, are you sure that this should go into Core? Why not make this a
> separate module? It could go directly into Eigen/ skipping
> unsupported/ as it's obvious that there are many users interested in
> this.
>
> I'd rather have this as a separate module (not a very strong opinion
> though) and I don't remember a decision on this being made, perhaps
> this is something you already discussed with Gael?
Since we are adding types, and not functions like cwise(), QR, SVD
etc., I'd prefer to have it in Core, merely for the sake of
consistency.

> Second, your unit test is not enough. It should at least cover basic
> matrix arithmetic. So a good starting point for a unit test is to copy
> and adapt large parts of basicstuff.cpp and/or linearstructure.cpp.

Sure, will do that.
> When you do vectorization, add also packetmath.cpp. Keep all that
> templated, so it can easily be extended to cover more new small
> integer types. Just wipe what is not wanted / not applicable for small
> integer types.
Actually, I have looked at char and uchar support in SSE2 as well.
Let's just say that we have a SNAFU (thank you, Intel, :P ) here.

> Do you want write access to eigen2 so you can eventually push
> yourself? (nontrivial additions into Core, especially, still need to
> be discussed on this list)
Well, not yet. I'd rather have a complete set of unit-tests before pushing.

Thanks,

-- 
Rohit Garg

http://rpg-314.blogspot.com/

Senior Undergraduate
Department of Physics
Indian Institute of Technology
Bombay