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

[Jules Bergmann <jules@xxxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxxx
(650) 331-3385 x705