kokkos / kokkos-kernels

Kokkos C++ Performance Portability Programming Ecosystem: Math Kernels - Provides BLAS, Sparse BLAS and Graph Kernels
Other
303 stars 96 forks source link

axpby conditional for layout left/right #341

Open jjwilke opened 5 years ago

jjwilke commented 5 years ago

The code for axpby chooses an algorithm with std::conditional:

typedef typename std::conditional<
   std::is_same<typename XMV::array_layout,Kokkos::LayoutLeft>::value,
   Axpby_MV_Invoke_Right<AV, XMV, BV, YMV, index_type>,
   Axpby_MV_Invoke_Left<AV, XMV, BV, YMV, index_type> 
>::type Axpby_MV_Invoke_Layout;
  Axpby_MV_Invoke_Layout::run(alpha, X, beta, Y, a, b);

Unless I'm misreading the conditional, this is:

if (LayoutLeft) Axpby_MV_Invoke_Right(...)
if (LayoutOther) Axpby_MV_Invoke_Left(...)

The comments indicate the the Invoke_Right version is intended for XMV with layout right... and the implementation seems to match that (CPU version, assign whole row to thread) where as the layout left version unrolls (GPU version).

Should the conditional be flipped?

ndellingwood commented 5 years ago

@jjwilke have you had a chance to confirm your suspicion that the conditional is incorrect?

jjwilke commented 5 years ago

Not yet. I foolishly believed I could quickly update CMake and then use the new CMake standalone for running my tests.

ndellingwood commented 5 years ago

@jjwilke I'll send an email so we can coordinate getting your cmake changes in kokkos and kokkos-kernels, sorry for the delay.