Skip to content

Extend DispatchStub to support CUDA dispatch #9664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
colesbury wants to merge 2 commits into from
Closed

Extend DispatchStub to support CUDA dispatch #9664

colesbury wants to merge 2 commits into from

Conversation

@colesbury
Copy link
Member

@colesbury colesbury commented Jul 20, 2018 •
edited
Loading

This is a modification of the strategy from #8919 and #9579.

Previously, the CPU architecture-specific kernels self-registered with
the DispatchStub. When linking as part of a static library, this requires
the flag --whole-archive to be passed to the linker to ensure that the
object files for the kernels are included. Caffe2 and TensorFlow use that
strategy.

We ran into some issues with --whole-archive blowing up the binary size
of some downstream projects in Facebook. This PR avoids --whole-archive
for CPU kernels. The downside is that the generic code needs to be aware
of whether kernels are compiled with AVX and with AVX2 (via
HAVE_AVX_CPU_DEFINITION and HAVE_AVX2_CPU_DEFINITION).

The CUDA kernels still self-register with DispatchStub because the CPU
library is not aware of whether the CUDA library will be available at
runtime.

There are a few major changes to DispatchStub

 - The environment variable ATEN_CPU_CAPABILITY overrides the CPU
   capability detection code (Previous ATEN_DISABLE_AVX/AVX2)

 - DispatchStub is defined in the generic native code instead of the
   CPU_CAPABILITY_DEFAULT kernel.

facebook-github-bot
facebook-github-bot reviewed Jul 20, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colesbury has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@colesbury
Extend DispatchStub to support CUDA dispatch
52152a8 
This is a modification of the strategy from
pytorch#8919 and
pytorch#9579.

Previously, the CPU architecture-specific kernels self-registered with
the DispatchStub. When linking as part of a static library, this requires
the flag --whole-archive to be passed to the linker to ensure that the
object files for the kernels are included. Caffe2 and TensorFlow use that
strategy.

We ran into some issues with --whole-archive blowing up the binary size
of some downstream projects in Facebook. This PR avoids --whole-archive
for CPU kernels. The downside is that the generic code needs to be aware
of whether kernels are compiled with AVX and with AVX2 (via
HAVE_AVX_CPU_DEFINITION and HAVE_AVX2_CPU_DEFINITION).

The CUDA kernels still self-register with DispatchStub because the CPU
library is not aware of whether the CUDA library will be available at
runtime.

There are a few major changes to DispatchStub

 - The environment variable ATEN_CPU_CAPABILITY overrides the CPU
   capability detection code (Previous ATEN_DISABLE_AVX/AVX2)

 - DispatchStub is defined in the generic native code instead of the
   CPU_CAPABILITY_DEFAULT kernel.
@soumith
Copy link
Contributor

soumith commented Jul 21, 2018

@pytorchbot retest this please

@ezyang
Copy link
Contributor

ezyang commented Jul 23, 2018

Build error looks real:

05:21:39 /private/var/lib/jenkins/workspace/pytorch-builds/pytorch-macos-10.13-py3-test2/aten/src/ATen/native/DispatchStub.h:72:18: error: instantiation of variable 'at::native::DispatchStub<void (*)(at::Tensor &, const at::Tensor &), at::native::softmax_lastdim_kernel>::AVX' required here, but no definition is available [-Werror,-Wundefined-var-template]
05:21:39       AT_ASSERTM(AVX, "DispatchStub: missing AVX kernel");
05:21:39                  ^
05:21:39 /private/var/lib/jenkins/workspace/pytorch-builds/pytorch-macos-10.13-py3-test2/aten/src/ATen/native/DispatchStub.h:50:28: note: in instantiation of member function 'at::native::DispatchStub<void (*)(at::Tensor &, const at::Tensor &), at::native::softmax_lastdim_kernel>::choose_cpu_impl' requested here
05:21:39         cpu_dispatch_ptr = choose_cpu_impl();
05:21:39                            ^
05:21:39 /private/var/lib/jenkins/workspace/pytorch-builds/pytorch-macos-10.13-py3-test2/aten/src/ATen/native/SoftMax.cpp:131:27: note: in instantiation of function template specialization 'at::native::DispatchStub<void (*)(at::Tensor &, const at::Tensor &), at::native::softmax_lastdim_kernel>::operator()<at::Tensor, at::Tensor>' requested here
05:21:39     softmax_lastdim_kernel(kCPU, output, input);
05:21:39                           ^
05:21:39 /private/var/lib/jenkins/workspace/pytorch-builds/pytorch-macos-10.13-py3-test2/aten/src/ATen/native/DispatchStub.h:84:16: note: forward declaration of template entity is here
05:21:39   static FnPtr AVX;
05:21:39                ^
05:21:39 /private/var/lib/jenkins/workspace/pytorch-builds/pytorch-macos-10.13-py3-test2/aten/src/ATen/native/DispatchStub.h:72:18: note: add an explicit instantiation declaration to suppress this warning if 'at::native::DispatchStub<void (*)(at::Tensor &, const at::Tensor &), at::native::softmax_lastdim_kernel>::AVX' is explicitly instantiated in another translation unit
05:21:39       AT_ASSERTM(AVX, "DispatchStub: missing AVX kernel");

facebook-github-bot
facebook-github-bot reviewed Jul 23, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colesbury has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

gchanan
gchanan approved these changes Jul 23, 2018
View reviewed changes
Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Jul 23, 2018
@colesbury @facebook-github-bot
Extend DispatchStub to support CUDA dispatch (#9664)
f48ef7a 
Summary:
This is a modification of the strategy from pytorch/pytorch#8919 and pytorch/pytorch#9579.

```
Previously, the CPU architecture-specific kernels self-registered with
the DispatchStub. When linking as part of a static library, this requires
the flag --whole-archive to be passed to the linker to ensure that the
object files for the kernels are included. Caffe2 and TensorFlow use that
strategy.

We ran into some issues with --whole-archive blowing up the binary size
of some downstream projects in Facebook. This PR avoids --whole-archive
for CPU kernels. The downside is that the generic code needs to be aware
of whether kernels are compiled with AVX and with AVX2 (via
HAVE_AVX_CPU_DEFINITION and HAVE_AVX2_CPU_DEFINITION).

The CUDA kernels still self-register with DispatchStub because the CPU
library is not aware of whether the CUDA library will be available at
runtime.

There are a few major changes to DispatchStub

 - The environment variable ATEN_CPU_CAPABILITY overrides the CPU
   capability detection code (Previous ATEN_DISABLE_AVX/AVX2)

 - DispatchStub is defined in the generic native code instead of the
   CPU_CAPABILITY_DEFAULT kernel.
```
Pull Request resolved: pytorch/pytorch#9664

Differential Revision: D8943350

Pulled By: colesbury

fbshipit-source-id: 329229b0ee9ff94fc001b960287814bd734096ef
jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 30, 2018
@colesbury @jramseyer
Extend DispatchStub to support CUDA dispatch (pytorch#9664)
71689e9 
Summary:
This is a modification of the strategy from pytorch#8919 and pytorch#9579.

```
Previously, the CPU architecture-specific kernels self-registered with
the DispatchStub. When linking as part of a static library, this requires
the flag --whole-archive to be passed to the linker to ensure that the
object files for the kernels are included. Caffe2 and TensorFlow use that
strategy.

We ran into some issues with --whole-archive blowing up the binary size
of some downstream projects in Facebook. This PR avoids --whole-archive
for CPU kernels. The downside is that the generic code needs to be aware
of whether kernels are compiled with AVX and with AVX2 (via
HAVE_AVX_CPU_DEFINITION and HAVE_AVX2_CPU_DEFINITION).

The CUDA kernels still self-register with DispatchStub because the CPU
library is not aware of whether the CUDA library will be available at
runtime.

There are a few major changes to DispatchStub

 - The environment variable ATEN_CPU_CAPABILITY overrides the CPU
   capability detection code (Previous ATEN_DISABLE_AVX/AVX2)

 - DispatchStub is defined in the generic native code instead of the
   CPU_CAPABILITY_DEFAULT kernel.
```
Pull Request resolved: pytorch#9664

Differential Revision: D8943350

Pulled By: colesbury

fbshipit-source-id: 329229b0ee9ff94fc001b960287814bd734096ef
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@colesbury
Extend DispatchStub to support CUDA dispatch (pytorch#9664)
ff5176b 
Summary:
This is a modification of the strategy from pytorch#8919 and pytorch#9579.

```
Previously, the CPU architecture-specific kernels self-registered with
the DispatchStub. When linking as part of a static library, this requires
the flag --whole-archive to be passed to the linker to ensure that the
object files for the kernels are included. Caffe2 and TensorFlow use that
strategy.

We ran into some issues with --whole-archive blowing up the binary size
of some downstream projects in Facebook. This PR avoids --whole-archive
for CPU kernels. The downside is that the generic code needs to be aware
of whether kernels are compiled with AVX and with AVX2 (via
HAVE_AVX_CPU_DEFINITION and HAVE_AVX2_CPU_DEFINITION).

The CUDA kernels still self-register with DispatchStub because the CPU
library is not aware of whether the CUDA library will be available at
runtime.

There are a few major changes to DispatchStub

 - The environment variable ATEN_CPU_CAPABILITY overrides the CPU
   capability detection code (Previous ATEN_DISABLE_AVX/AVX2)

 - DispatchStub is defined in the generic native code instead of the
   CPU_CAPABILITY_DEFAULT kernel.
```
Pull Request resolved: pytorch#9664

Differential Revision: D8943350

Pulled By: colesbury

fbshipit-source-id: 329229b0ee9ff94fc001b960287814bd734096ef
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@gchanan gchanan gchanan approved these changes

@apaszke apaszke Awaiting requested review from apaszke

@ezyang ezyang Awaiting requested review from ezyang

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@colesbury @soumith @ezyang @gchanan @facebook-github-bot