[vsipl++] Index and Length
Jules Bergmann
jules at codesourcery.com
Mon Apr 3 23:25:34 UTC 2006
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.
>
--
Jules Bergmann
CodeSourcery
jules at codesourcery.com
(650) 331-3385 x705
More information about the vsipl++
mailing list