[vsipl++] par evaluators

[Jules Bergmann]

Assem Salama wrote:
 > Everyone,
 >  This patch address Jules comments.

Assem,

There are several items from my last feedback that you did not
address:

---

[1] When you adding to existing code, try to follow the lead set by it
if possible (and if it doesn't violate our coding standards :) .  Here
the '#includes" are indented to improve readability of the #ifdef
logic.  Your new include should be indented too.

---

(for global_from_local_index_blk:)

[2] What namespace is this in?  vsip or vsip::impl ?

If it is in vsip, please move it to vsip::impl.

(Can you answer the question?  Is it in vsip or vsip::impl?)

---

[3]                  ^^^^ opt

---

Also, I have some additional feedback below.

				-- Jules



 > ------------------------------------------------------------------------
 >
 > Index: benchmarks/maxval.cpp
 > ===================================================================

 > @@ -96,6 +173,12 @@
 >    case  1: loop(t_maxval1<float>(0)); break;
 >    case  2: loop(t_maxval1<float>(1)); break;
 >    case  3: loop(t_maxval1<float>(2)); break;
 > +  case  4: loop(t_maxval2<float,Map<>,impl::Parallel_tag>(0)); break;
 > +  case  5: loop(t_maxval2<float,Map<>,impl::Parallel_tag>(1)); break;
 > +  case  6: loop(t_maxval2<float,Map<>,impl::Parallel_tag>(2)); break;

 > +  case  7: loop(t_maxval2<float,Map<>,impl::Cvsip_tag>(0)); break;
 > +  case  8: loop(t_maxval2<float,Map<>,impl::Cvsip_tag>(1)); break;
 > +  case  9: loop(t_maxval2<float,Map<>,impl::Cvsip_tag>(2)); break;

[1] Will this benchmark build if the C-VSIP backend is not configured
in?  If not, please guard these cases with an ifdef.


 >    default: return 0;
 >    }
 >    return 1;
 > Index: benchmarks/maxval.hpp
 > ===================================================================
 > --- benchmarks/maxval.hpp	(revision 0)
 > +++ benchmarks/maxval.hpp	(revision 0)
 > @@ -0,0 +1,101 @@
 > +/* Copyright (c) 2006 by CodeSourcery.  All rights reserved.

[2] Please fix the date.

 > +
 > +   This file is available for license from CodeSourcery, Inc. under 
the terms
 > +   of a commercial license and under the GPL.  It is not part of the 
VSIPL++
 > +   reference implementation and is not available under the BSD license.
 > +*/
 > +/** @file    benchmarks/maxval.hpp
 > +    @author  Assem Salama
 > +    @date    2006-07-22

[3] Please fix the date.

 > +    @brief   VSIPL++ Library: Helper file for maxval benchmark
 > +
 > +*/
 > +#ifndef BENCHMARKS_MAXVAL_HPP
 > +#define BENCHMARKS_MAXVAL_HPP
 > +
 > +using namespace vsip::impl;
 > +using namespace vsip;

[4] Header files shouldn't have global 'using namespace' statements
that import names into the global namespace.  They can introduces
order-dependent behavior (names introduced before this header will be
included, names introduced after will not).

Instead, names should either have an explicit namespace
(i.e. vsip::dimension_type), or 'using' statements should have limited
scope (such as within a function).




 > Index: src/vsip/opt/reductions/par_reductions.hpp
 > ===================================================================


 > +/***********************************************************************
 > +  Parallel evaluators.

[5] Please update or remove this comment.

 > +***********************************************************************/



 > +/**********************************************************************
 > +* Parallel evaluators for index returning reductions

[6] Please change this to the singular "Parallel evaluator".  There is
only one evaluator.

 > +**********************************************************************/
 > +
 > +template <template <typename> class ReduceT,
 > +          typename                  T,
 > +	  typename                  Block,
 > +	  typename                  OrderT,
 > +	  dimension_type            Dim >
 > +struct Evaluator<Op_reduce_idx<ReduceT>, T,
 > +		 Op_list_3<Block const&, Index<Dim>&, OrderT>,
 > +		 Parallel_tag>


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