|
|
Blitz Devel : |
From: Todd Veldhuizen (tveldhui_at_[hidden])
Date: 2002-10-11 11:09:55
I agree with Julian, that it would be good to get rid of some of the
compiler kludges like _bz_typename, but that it makes sense to do this
only so far as it doesn't break support of compilers. It should be a
fairly simple thing to line up some config.h's from various compilers
on the "supported compilers" list (I guess we'd have to decide what
that list is..) and figure out which kludges may be safely removed.
Cheers,
Todd
Julian Cummings wrote:
>
> 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
>
-- Todd Veldhuizen / tveldhui_at_[hidden] / Indiana University Computer Science _______________________________________________ Blitz-dev mailing list Blitz-dev_at_[hidden] http://www.oonumerics.org/mailman/listinfo.cgi/blitz-dev