[vsipl++] Index and Length
Assem Salama
assem at codesourcery.com
Tue Apr 4 02:23:24 UTC 2006
Jules,
Applied suggested changes and checked them in.
Assem Salama
Jules Bergmann wrote:
> Assem Salama wrote:
>> Everyone,
>> This patch completely removes the use of Point in the library. Index
>> and Length are now used instead.
>>
>> Thanks,
>> Assem Salama
>
> Assem,
>
> This patch looks good. I have several comments below, please check it
> in after fixing them.
>
> thanks,
> -- Jules
>
>>
>>
>> ------------------------------------------------------------------------
>>
>
>> Index: src/vsip/dense.hpp
>> ===================================================================
>> RCS file: /home/cvs/Repository/vpp/src/vsip/dense.hpp,v
>> retrieving revision 1.35
>> diff -u -r1.35 dense.hpp
>> --- src/vsip/dense.hpp 27 Mar 2006 23:19:34 -0000 1.35
>> +++ src/vsip/dense.hpp 3 Apr 2006 15:26:15 -0000
>> @@ -23,7 +23,7 @@
>> #include <vsip/impl/layout.hpp>
>> #include <vsip/impl/extdata.hpp>
>> #include <vsip/impl/block-traits.hpp>
>> -#include <vsip/impl/point.hpp>
>> +#include <vsip/domain.hpp>
>>
>> /// Complex storage format for dense blocks.
>> #if VSIP_IMPL_PREFER_SPLIT_COMPLEX
>> @@ -33,6 +33,7 @@
>> #endif
>>
>>
>> +using vsip::Index;
>
> This isn't necessary. The uses of Index in the body of dense.hpp are
> inside of a "namespace vsip { ... }" block, so they will see
> vsip::Index fine.
>
> Moreover, putting this 'using' statement here will put 'Index' in the
> top-level namespace for user programs, which we're not allowed to do.
>
>>
>> /***********************************************************************
>>
>> Declarations
>> @@ -536,8 +537,8 @@
>>
>> protected:
>> // Dim-dimensional get/put
>> - T get(Point<Dim> const& idx) const VSIP_NOTHROW;
>> - void put(Point<Dim> const& idx, T val) VSIP_NOTHROW;
>> + T get(Index<Dim> const& idx) const VSIP_NOTHROW;
>> + void put(Index<Dim> const& idx, T val) VSIP_NOTHROW;
>>
>> // 2-diminsional get/put
>> T impl_get(index_type idx0, index_type idx1) const VSIP_NOTHROW
>> @@ -558,8 +559,8 @@
>>
>> protected:
>> // Dim-dimensional lvalue.
>> - reference_type impl_ref(Point<Dim> const& idx) VSIP_NOTHROW;
>> - const_reference_type impl_ref(Point<Dim> const& idx) const
>> VSIP_NOTHROW;
>> + reference_type impl_ref(Index<Dim> const& idx) VSIP_NOTHROW;
>> + const_reference_type impl_ref(Index<Dim> const& idx) const
>> VSIP_NOTHROW;
>>
>> // Accessors.
>> public:
>> @@ -779,11 +780,11 @@
>>
>> reference_type impl_ref(index_type idx0, index_type idx1)
>> VSIP_NOTHROW
>> - { return base_type::impl_ref(impl::Point<2>(idx0, idx1)); }
>> + { return base_type::impl_ref(Index<2>(idx0, idx1)); }
>>
>> const_reference_type impl_ref(index_type idx0, index_type idx1)
>> const VSIP_NOTHROW
>> - { return base_type::impl_ref(impl::Point<2>(idx0, idx1)); }
>> + { return base_type::impl_ref(Index<2>(idx0, idx1)); }
>> };
>>
>>
>> @@ -901,12 +902,12 @@
>>
>> reference_type impl_ref(index_type idx0, index_type idx1,
>> index_type idx2)
>> VSIP_NOTHROW
>> - { return base_type::impl_ref(impl::Point<3>(idx0, idx1, idx2)); }
>> + { return base_type::impl_ref(Index<3>(idx0, idx1, idx2)); }
>>
>> const_reference_type impl_ref(index_type idx0, index_type idx1,
>> index_type idx2)
>> const VSIP_NOTHROW
>> - { return base_type::impl_ref(impl::Point<3>(idx0, idx1, idx2)); }
>> + { return base_type::impl_ref(Index<3>(idx0, idx1, idx2)); }
>> };
>>
>>
>> @@ -1329,7 +1330,7 @@
>> inline
>> T
>> Dense_impl<Dim, T, OrderT, MapT>::get(
>> - Point<Dim> const& idx)
>> + Index<Dim> const& idx)
>> const VSIP_NOTHROW
>> {
>> for (dimension_type d=0; d<Dim; ++d)
>> @@ -1346,7 +1347,7 @@
>> inline
>> void
>> Dense_impl<Dim, T, OrderT, MapT>::put(
>> - Point<Dim> const& idx,
>> + Index<Dim> const& idx,
>> T val)
>> VSIP_NOTHROW
>> {
>> @@ -1364,7 +1365,7 @@
>> inline
>> typename Dense_impl<Dim, T, OrderT, MapT>::reference_type
>> Dense_impl<Dim, T, OrderT, MapT>::impl_ref(
>> - Point<Dim> const& idx) VSIP_NOTHROW
>> + Index<Dim> const& idx) VSIP_NOTHROW
>> {
>> for (dimension_type d=0; d<Dim; ++d)
>> assert(idx[d] < layout_.size(d));
>> @@ -1380,7 +1381,7 @@
>> inline
>> typename Dense_impl<Dim, T, OrderT, MapT>::const_reference_type
>> Dense_impl<Dim, T, OrderT, MapT>::impl_ref(
>> - Point<Dim> const& idx) const VSIP_NOTHROW
>> + Index<Dim> const& idx) const VSIP_NOTHROW
>> {
>> for (dimension_type d=0; d<Dim; ++d)
>> assert(idx[d] < layout_.size(d));
>> Index: src/vsip/domain.hpp
>> ===================================================================
>> RCS file: /home/cvs/Repository/vpp/src/vsip/domain.hpp,v
>> retrieving revision 1.15
>> diff -u -r1.15 domain.hpp
>> --- src/vsip/domain.hpp 19 Sep 2005 03:39:54 -0000 1.15
>> +++ src/vsip/domain.hpp 3 Apr 2006 15:26:15 -0000
>> @@ -31,6 +31,8 @@
>> Index(index_type x) VSIP_NOTHROW : Vertex<index_type, 1>(x) {}
>> };
>>
>> +// mathematical operations for Index
>> +/*
>> inline bool operator==(Index<1> const& i, Index<1> const& j)
>> VSIP_NOTHROW
>> {
>> @@ -38,6 +40,46 @@
>> static_cast<Vertex<index_type, 1> >(i) ==
>> static_cast<Vertex<index_type, 1> >(j);
>> }
>> +*/
>
> If you comment code out, you should include some reason why it is
> commented out to avoid confusion.
>
> In this case, there is no reason to keep the old code around, so you
> should just remove it.
>
>> +
>> +template <dimension_type Dim>
>> +inline bool +operator==(Index<Dim> const& i, Index<Dim> const& j)
>> VSIP_NOTHROW
>> +{
>> + for (dimension_type d=0; d<Dim; ++d)
>> + if (i[d] != j[d])
>> + return false;
>> + return true;
>> +}
>
> This looks good.
>
>
>> {
>> Index: src/vsip/matrix.hpp
>> ===================================================================
>> RCS file: /home/cvs/Repository/vpp/src/vsip/matrix.hpp,v
>> retrieving revision 1.30
>> diff -u -r1.30 matrix.hpp
>> --- src/vsip/matrix.hpp 11 Jan 2006 16:22:44 -0000 1.30
>> +++ src/vsip/matrix.hpp 3 Apr 2006 15:26:15 -0000
>> @@ -401,6 +401,18 @@
>> return Domain<2>(view.size(0), view.size(1));
>> }
>>
>> +/// Get the extent of a matrix view, as a Length.
>> +
>> +template <typename T,
>> + typename Block>
>> +Length<2>
>
> This should be 'inline'. Small template functions like this in header
> files should be declared inline. It improves efficiency, and some
> compilers (such as greenhills) have trouble with leaving them
> non-inline functions.
>
> We haven't 100% good about following this rule. When you come across
> a small non-inline template function in a header, chances are it
> should be 'inline'.
>
>> +extent(const_Matrix<T, Block> v)
>> +{
>> + return Length<2>(v.size(0), v.size(1));
>> +}
>> +
>> +
>> +
>> } // namespace vsip::impl
>>
>> } // namespace vsip
>> Index: src/vsip/vector.hpp
>> ===================================================================
>> RCS file: /home/cvs/Repository/vpp/src/vsip/vector.hpp,v
>> retrieving revision 1.38
>> diff -u -r1.38 vector.hpp
>> --- src/vsip/vector.hpp 22 Mar 2006 20:48:58 -0000 1.38
>> +++ src/vsip/vector.hpp 3 Apr 2006 15:26:15 -0000
>> @@ -354,6 +354,18 @@
>> return Domain<1>(view.size(0));
>> }
>>
>> +/// Get the extent of a vector view, as a Length. +
>> +template <typename T,
>> + typename Block>
>> +Length<1>
>
> this should be 'inline'
>
>> +extent(const_Vector<T, Block> v)
>> +{
>> + return Length<1>(v.size(0));
>> +}
>> +
>> +
>> +
>> } // namespace vsip::impl
>>
>>
>
>>
>> ------------------------------------------------------------------------
>>
>> 2006-04-03 Assem Salama <assem at codesourcery.com>
>> * src/vsip/dense.hpp: Converted this file to use Index and Length
>> instead of Point.
>> * src/vsip/matrix.hpp: Same as above.
>> * src/vsip/vector.hpp: Same as above.
>> * src/vsip/impl/block-copy.hpp: Same as above.
>> * src/vsip/impl/extdata.hpp: Same as above.
>> * src/vsip/impl/fast-block.hpp: Same as above.
>> * src/vsip/impl/lvalue-proxy.hpp: Same as above.
>> * src/vsip/impl/par-assign.hpp: Same as above.
>> * src/vsip/impl/par-chain-assign.hpp: Same as above.
>> * src/vsip/impl/par-foreach.hpp: Same as above.
>> * src/vsip/impl/layout.hpp: Same as above. Had to change index
>> index functions to take Index instead of Point.
>> * src/vsip/domain.hpp: Added operators ==,-,and + for Index.
>> * src/vsip/impl/domain-utils.hpp: Added extent functions that return
>> Length instead of point.
>> * src/vsip/impl/par-util.hpp: Changed the foreach_point function to
>> work on Index instead of Point.
>> * src/vsip/impl/point-fcn.hpp: Removed this file from cvs. The
>> use of
>> Point is deprecated. We now use Index and Length.
>> * src/vsip/impl/point.hpp: Removed this from cvs. We now use
>> Length and
>> Index instead of Point.
>
> comments look good, just check that they fit into 80 columns
>
>> * tests/output.hpp: Changed the << operator to operate on an Index.
>> * tests/appmap.cpp: Converted this test to use Length and Index.
>> * tests/fast-block.cpp: Same as appmap.cpp
>> * tests/us-block.cpp: Same as above.
>> * tests/user_storage.cpp: Same as above.
>> * tests/util-par.hpp: Same as above.
>> * tests/view.cpp: Same as above.
>> * tests/vmmul.cpp: Same as above.
>> * tests/parallel/block.cpp: Same as above.
>> * tests/parallel/expr.cpp: Same as above.
>> * tests/parallel/subviews.cpp: Same as above.
>>
>
>
More information about the vsipl++
mailing list