[vsipl++] [patch] Matrix product 'backend' for CML

[Jules Bergmann]

Don McCoy wrote:
 > Don McCoy wrote:
 >> I changed the parameter to accept the block type rather than the
 >> value_type for the block.  The advantage here is that it centralizes the
 >> ...
 >>
 > One minor change: it now checks all three blocks, as suggested off list.

Don,

This looks good to me.  I have a couple of minor comments below,
otherwise, please check it in.

				thanks,
				-- Jules



 > +template <typename Block0,
 > +          typename Block1,
 > +          typename Block2>
 > +struct Evaluator<Op_prod_mm, Block0, Op_list_2<Block1, Block2>,
 > +                 Cml_tag>
 > +{
 > +  typedef typename Block0::value_type T;
 > +  typedef typename Block_layout<Block0>::order_type order0_type;
 > +  typedef typename Block_layout<Block1>::order_type order1_type;
 > +  typedef typename Block_layout<Block2>::order_type order2_type;
 > +
 > +  static bool const ct_valid =
 > +    // check that CML supports this data type and/or layout
 > +    impl::cml::Cml_supports_block<Block0>::valid &&
 > +    impl::cml::Cml_supports_block<Block1>::valid &&
 > +    impl::cml::Cml_supports_block<Block2>::valid &&
 > +    // check that all data types are equal
 > +    Type_equal<T, typename Block1::value_type>::value &&
 > +    Type_equal<T, typename Block2::value_type>::value &&
 > +    // check that the layout is row-major
 > +    Type_equal<order0_type, row2_type>::value &&
 > +    Type_equal<order1_type, row2_type>::value &&
 > +    Type_equal<order2_type, row2_type>::value &&
 > +    // check that direct access is supported
 > +    Ext_data_cost<Block0>::value == 0 &&
 > +    Ext_data_cost<Block1>::value == 0 &&
 > +    Ext_data_cost<Block2>::value == 0;

You should also check that all three blocks have the same
split/interleave format (using Is_split_block).  Otherwise, this would
try to handle a product between a split-complex and
interleaved-complex block.



 > +// This version is for split complex only.
 > +
 > +#define VSIP_IMPL_CML_ZMPROD(T, FCN)    \
 > +inline void                             \
 > +mprod(                                  \
 > +  std::pair<T, T> a, int lda,           \
 > +  std::pair<T, T> b, int ldb,           \
 > +  std::pair<T, T> z, int ldz,           \

Its a minor point, but I was expecting to see 'std::pair<T*, T*>'.  I
realize you pass 'T=float*' so it works out.

For consistentency with the other macro I would suggest putting the
'*' in the macro and passing 'T=float'.

 > +VSIP_IMPL_CML_ZMPROD(float*, cml_zmprod1_f)
 > +#undef VSIP_IMPL_CML_ZMPROD


-- 
Jules Bergmann
CodeSourcery
jules at codesourcery.com
(650) 331-3385 x705