[vsipl++] [patch] Scalable SAR benchmark

[Jules Bergmann]

 > This patch addresses comments 4, 6, and 7 (diffview, fft_shift
 > implementation), in addition to some cleanups that allow it to run with
 > the current reorganized code base.

Don,

This looks good, I have three suggestions below, otherwise it looks
good to check in.

 >
 > BTW, the change in the fftshift function resulted in a 2.5x speedup!

Sweet!

				thanks,
				-- Jules

 > ------------------------------------------------------------------------
 >
 > Index: src/vsip_csl/matlab_utils.hpp
 > ===================================================================

 > +// The following versions are not as efficient as those above due
 > +// to the overhead of creating a new view.  For optimized code,
 > +// use the ones above.
 > +
 > +template <typename T1,
 > +          typename Block1>
 > +Vector<T1, Block1>
 > +fftshift(
 > +  const_Vector<T1, Block1> in)
 > +{

You could define these by-value versions in terms of the above
by-reference versions:

      Vector<T1> out(nx);
      fftshift(in, out);
      return out;
    }

Also, because you don't know what Block1 is, you should use
'Vector<T1>' (defaulting to a Dense block) instead of 'Vector<T1,
Block1>'.  For example, Block1 could be a subblock type which can't be
used in this context.  This affects both the defn of out, and the
return type of the function.

 > +  // This function swaps halves of a vector (dimension
 > +  // must be even).
 > +
 > +  length_type nx = in.size(0);
 > +  assert(!(nx & 1));
 > +  assert(nx == out.size(0));
 > +
 > +  Domain<1> left(0, 1, nx/2);
 > +  Domain<1> right(nx/2, 1, nx/2);
 > +
 > +  Vector<T1, Block1> out(nx);
 > +  out(left) = in(right);
 > +  out(right) = in(left);
 > +
 > +  return out;
 > +}
 > +
 > +
 > +template <typename T1,
 > +	  typename Block1>
 > +Matrix<T1, Block1>
 > +fftshift(
 > +  const_Matrix<T1, Block1> in)
 > +{

Likewise.




 > Index: apps/ssar/kernel1.hpp
 > ===================================================================

 > @@ -225,15 +221,16 @@
 >    // (spatial frequency) domain.
 >
 >    // 59. (n by mc array of complex numbers) filtered echoed signal
 > -  s_filt_ = vmmul<col>(fast_time_filter_, 
col_fftm_(this->fft_shift(s_raw_)));
 > +  s_filt_ = vmmul<col>(fast_time_filter_,
 > +    col_fftm_(vsip_csl::matlab::fftshift(s_raw_, s_filt_)));

You could use a 'using vsip_csl::matlab::fftshift' in the function
body.



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