Enabling a debug dll build under Windows (without CUDA, at least)

[MikhailStartsev]

Modified SpaceToDepthOp and DepthToSpaceOp templated classes not to use a SpaceToDepthOpFunctor/DepthToSpaceOpFunctor structs with a template parameter Device=_GPU_Device in case the class itself is instantiated with Device=_CPU_Device. Added a partial template specialization for Device=GPUDevice to preserve the existing behaviour in all cases.

This at least partially (i.e. when bazel is configured not to use CUDA) fixes issue #41118.

[MikhailStartsev]

The order of includes changed according to clang-formatting tool with -style=google

[MikhailStartsev]

@rthadur Re-opening PR #42268 with master branch this time.

[sanjoy]

End comment with period, also typo in referncing

[MikhailStartsev]

Done

[sanjoy]

Can we specialize just this part that's different between GPU and CPU? Same comment for SpaceToDepthOp.

[MikhailStartsev]

That's what I tried to do initially - specialize only the Compute() function, but AFAIU there is no way to specialize part of the class definition only: The template signature of the function has to match that of the class, so to partially specialize the function you need to partially specialize the class.

Taking the GPU-specialized code into a separate function would result in the same situation - the class still needs to be specialized.

One could probably get around this with some inheritance trickery, but this IMO is making this messier than it should be. Not that I like this code duplication much more...

[sanjoy]

I mean something like:

template <class Device>
struct DepthToSpaceFunctorWrapper {
  void operator(params) {
    // .. generic implementation
  }
};

template <>
struct DepthToSpaceFunctorWrapper<GPUDevice> {
  void operator(params) {
    // .. GPU implementation
  }
};

void Compute(...) {
  // Common stuff ..
  auto Toutput = outputs_tensor->tensor<T, kDims>();
  DepthToSpaceFunctorWrapper<Device>{}(params as needed);
}

[MikhailStartsev]

Wait, isn't there a much simpler solution? In the if clause here, if one is inside it, it is guaranteed that Device is GPUDevice. So why not replace functor::DepthToSpaceOpFunctor<GPUDevice, T, FORMAT_NCHW> functor; with functor::DepthToSpaceOpFunctor<Device, T, FORMAT_NCHW> functor; inside this clause? This should be identical, no? If the condition is false, we never get into the code with the "wrong" device, if it is true - the code is literally the same.

[sanjoy]

I mean something like:

template <class Device>
struct DepthToSpaceFunctorWrapper {
  void operator(params) {
    // .. generic implementation
  }
};

template <>
struct DepthToSpaceFunctorWrapper<GPUDevice> {
  void operator(params) {
    // .. GPU implementation
  }
};

void Compute(...) {
  // Common stuff ..
  auto Toutput = outputs_tensor->tensor<T, kDims>();
  DepthToSpaceFunctorWrapper<Device>{}(params as needed);
}

[MikhailStartsev]

Please check out the much more concise change version :)

UPD: Hm. It's more concise alright, but the debug build now does not work again. It used to complain about missing symbols for the () operator of SpaceToDepthOpFunctor<GPUDevice, ...>, now I got same linker errors for SpaceToDepthOpFunctor<CPUDevice, ..., FORMAT_NCHW> because the CPUDevice-functor is only instantiated for FORMAT_NHWC as the last template argument...