|
|
Blitz Devel : |
From: Julian Cummings (cummings_at_[hidden])
Date: 2002-10-09 13:22:04
Hi,
Just a few comments on the "controversial" points below.
Theodore Papadopoulo wrote:
> Here is a patch I intend to commit tomorrow if no one opposes.
> It's main purpose is to remove some warning with unused arguments,
> to make the function swap work with Arrays.
>
> Three potentially controversial things are:
>
> + Removal of math.h if cmath is included, I do not see how it can
> hurt, but I'm only using one compiler ...
This should be OK as long as you have a "using namespace std;"
in there to bring the stuff declared in <cmath> into the current
enclosing namespace. I loathe such a using directive, but until
there is time to do a more comprehensive update to the blitz sources,
this will have to do. Eventually, I'd like to see most if not all of the
non-standard C++ hacks removed from blitz, and this includes using
the standard C++ header files and the std:: qualifier as needed.
> + Protection of _bz_intel_kludge with a __INTEL_COMPILER.
> Actually I'm not even sure that this is necessary with modern INTEL
> compilers (the bug is for version 4.0), this is certainly not perfect
> but at least it improves things. Can someone with an intel compiler
> test this ?
This is fine, although if the bug is really that old, perhaps you should
just remove the kludge entirely and see if anyone notices and complains.
> + I used typename directly instead of _bz_typename.
> Are there any modern compiler that does not accept the typename
> construct around ??? In my opinion, we should start to move towards
> using typename every where in template parameter definition.
> I notice that boost (that is tested againts several compilers) uses
> typename and class indifferently. Since (AFAIK), class is deprecated
> we should move towards typename.
The issue is with using typename to alert the compiler that the following
symbol is a type. This is usually done when the symbol is qualified by a
template type or something that is dependent on a template parameter.
For example, "typename std::vector<T>::iterator begin". It used to be
that many compilers did not yet require the presence of the typename
keyword here and in fact would get confused by it. So the _bz_typename
macro was used to control whether the keyword was present or not. In
the case you are talking about, using typename in a template parameter
definition (instead of class), the keyword is not optional. So I agree with
you that you should use typename rather than the _bz_typename macro
there. I don't know that using "class" here is deprecated. typename seems
a bit more logical and consistent, but Stroustrup recommends class because
it is shorter! Either one should be accepted by any reasonably current
compiler. I agree with you in principle about moving blitz towards standard
practices (see earlier comments), but my experience with this is that you
need to make such changes in one consistent sweep through the code. It
would be useful to examine the current state of each of the supported compilers
and see how many of these non-standard workarounds can be safely removed.
Regards, Julian C.
>
>
> Thank's
>
> Theo.
>
> 2002-10-08 Theodore Papadopoulo <Theodore.Papadopoulo_at_[hidden]>
> * blitz/memblock.h: Add support for swapping memblocks.
> * blitz/array-impl.h: Overload the swap function to work with
> Array.
> * blitz/blitz.h: Do not include math.h if cmath is included.
> * blitz/array/eval.cc: Move or remove unused statements.
> * blitz/traversal.cc: Protect _bz_intel_kludge with
> #ifdef __INTEL_COMPILER.
>
> Index: blitz/array-impl.h
> ===================================================================
> RCS file: /cvsroot/blitz/blitz/blitz/array-impl.h,v
> retrieving revision 1.10
> diff -c -3 -p -r1.10 array-impl.h
> *** blitz/array-impl.h 30 Aug 2002 22:12:49 -0000 1.10
> --- blitz/array-impl.h 9 Oct 2002 09:58:42 -0000
> *************** class IndirectArray;
> *** 162,167 ****
> --- 162,169 ----
>
> struct _bz_endTag {};
>
> + template <typename P_numtype, int N_rank>
> + void swap(Array<P_numtype,N_rank>&,Array<P_numtype,N_rank>&);
>
>
> /*
> *************** protected:
> *** 2473,2478 ****
> --- 2475,2482 ----
>
> void doTranspose(int destRank, int sourceRank, T_array& array);
>
> + friend void swap<>(Array<P_numtype,N_rank>&,Array<P_numtype,N_rank>&);
> +
> protected:
> //////////////////////////////////////////////
> // Data members
> *************** ostream& operator<<(ostream&, const Arra
> *** 2552,2557 ****
> --- 2556,2571 ----
>
> template<class T_numtype, int N_rank>
> istream& operator>>(istream& is, Array<T_numtype,N_rank>& x);
> +
> + template <typename P_numtype, int N_rank>
> + void swap(Array<P_numtype,N_rank>& a,Array<P_numtype,N_rank>& b) {
> + MemoryBlockReference<P_numtype>::swap(a,b);
> + std::swap(a.storage_,b.storage_);
> + std::swap(a.length_,b.length_);
> + std::swap(a.stride_,b.stride_);
> + std::swap(a.zeroOffset_,b.zeroOffset_);
> + }
> +
>
> BZ_NAMESPACE_END
>
> Index: blitz/blitz.h
> ===================================================================
> RCS file: /cvsroot/blitz/blitz/blitz/blitz.h,v
> retrieving revision 1.9
> diff -c -3 -p -r1.9 blitz.h
> *** blitz/blitz.h 19 Jul 2002 20:40:32 -0000 1.9
> --- blitz/blitz.h 9 Oct 2002 09:58:42 -0000
> ***************
> *** 115,123 ****
>
> #ifdef BZ_MATH_FN_IN_NAMESPACE_STD
> #include <cmath>
> #endif
> -
> - #include <math.h>
>
> #ifdef BZ_HAVE_COMPLEX
> #include <complex>
> --- 115,123 ----
>
> #ifdef BZ_MATH_FN_IN_NAMESPACE_STD
> #include <cmath>
> + #else
> + #include <math.h>
> #endif
>
> #ifdef BZ_HAVE_COMPLEX
> #include <complex>
> Index: blitz/memblock.h
> ===================================================================
> RCS file: /cvsroot/blitz/blitz/blitz/memblock.h,v
> retrieving revision 1.9
> diff -c -3 -p -r1.9 memblock.h
> *** blitz/memblock.h 19 Jul 2002 20:42:53 -0000 1.9
> --- blitz/memblock.h 9 Oct 2002 09:58:42 -0000
> *************** public:
> *** 382,387 ****
> --- 382,392 ----
> return block_->references();
> }
>
> + static void swap(MemoryBlockReference<P_type>& a,MemoryBlockReference<P_type>& b) {
> + std::swap(a.data_,b.data_);
> + std::swap(a.block_,b.block_);
> + }
> +
> protected:
>
> void changeToNullBlock()
> Index: blitz/traversal.cc
> ===================================================================
> RCS file: /cvsroot/blitz/blitz/blitz/traversal.cc,v
> retrieving revision 1.3
> diff -c -3 -p -r1.3 traversal.cc
> *** blitz/traversal.cc 6 Mar 2002 17:18:11 -0000 1.3
> --- blitz/traversal.cc 9 Oct 2002 09:58:42 -0000
> ***************
> *** 48,56 ****
> --- 48,58 ----
>
> BZ_NAMESPACE(blitz)
>
> + #ifdef __INTEL_COMPILER
> // Next line is a workaround for Intel C++ V4.0 oddity, due
> // to Allan Stokes.
> static set<TraversalOrder<2> > *_bz_intel_kludge;
> + #endif
>
> //template<int N_dimensions>
> //_bz_typename TraversalOrderCollection<N_dimensions>::T_set
> Index: blitz/array/eval.cc
> ===================================================================
> RCS file: /cvsroot/blitz/blitz/blitz/array/eval.cc,v
> retrieving revision 1.6
> diff -c -3 -p -r1.6 eval.cc
> *** blitz/array/eval.cc 27 May 2002 19:48:57 -0000 1.6
> --- blitz/array/eval.cc 9 Oct 2002 09:58:42 -0000
> *************** Array<T_numtype, N_rank>::evaluateWithSt
> *** 444,450 ****
> */
>
> const int maxRank = ordering(0);
> - const int secondLastRank = ordering(1);
>
> // Create an iterator for the array receiving the result
> FastArrayIterator<T_numtype, N_rank> iter(*this);
> --- 444,449 ----
> *************** Array<T_numtype, N_rank>::evaluateWithSt
> *** 567,575 ****
>
> if ((useUnitStride) || (useCommonStride))
> {
> - T_numtype * _bz_restrict end = const_cast<T_numtype*>(iter.data())
> - + lastLength;
> -
> #ifdef BZ_USE_FAST_READ_ARRAY_EXPR
>
> /*
> --- 566,571 ----
> *************** Array<T_numtype, N_rank>::evaluateWithSt
> *** 618,623 ****
> --- 614,622 ----
> // This bit of code not really needed; should remove at some
> // point, along with the test for BZ_USE_FAST_READ_ARRAY_EXPR
>
> + T_numtype * _bz_restrict end = const_cast<T_numtype*>(iter.data())
> + + lastLength;
> +
> while (iter.data() != end)
> {
> T_update::update(*const_cast<T_numtype*>(iter.data()), *expr);
> *************** Array<T_numtype, N_rank>::evaluateWithIn
> *** 736,744 ****
> // index traversal for the source expression
>
> const int maxRank = ordering(0);
> - const int secondLastRank = ordering(1);
>
> #ifdef BZ_DEBUG_TRAVERSE
> cout << "Index traversal: N_rank = " << N_rank << endl;
> cout << "maxRank = " << maxRank << " secondLastRank = " << secondLastRank
> << endl;
> --- 735,743 ----
> // index traversal for the source expression
>
> const int maxRank = ordering(0);
>
> #ifdef BZ_DEBUG_TRAVERSE
> + const int secondLastRank = ordering(1);
> cout << "Index traversal: N_rank = " << N_rank << endl;
> cout << "maxRank = " << maxRank << " secondLastRank = " << secondLastRank
> << endl;
> *************** cout.flush();
> *** 757,764 ****
>
> for (int i=0; i < N_rank; ++i)
> last(i) = storage_.base(i) + length_(i);
> -
> - int lastLength = length(maxRank);
>
> while (true) {
>
> --- 756,761 ----
>
> --------------------------------------------------------------------
> Theodore Papadopoulo
> Email: Theodore.Papadopoulo_at_[hidden] Tel: (33) 04 92 38 76 01
> --------------------------------------------------------------------
>
> _______________________________________________
> Blitz-dev mailing list
> Blitz-dev_at_[hidden]
> http://www.oonumerics.org/mailman/listinfo.cgi/blitz-dev
-- Dr. Julian C. Cummings E-mail: cummings_at_[hidden] California Institute of Technology Phone: 626-395-2543 1200 E. California Blvd., Mail Code 158-79 Fax: 626-584-5917 Pasadena, CA 91125 _______________________________________________ Blitz-dev mailing list Blitz-dev_at_[hidden] http://www.oonumerics.org/mailman/listinfo.cgi/blitz-dev