Skip to content

Conversation

@iotamudelta
Copy link
Contributor

This PR contains the ROCm contributions of last week:

  • documentation of pyHIPIFY data format originating from Add pyHIPIFY scripts needed for ROCm transpilation to PyTorch #8812 reviewing comments by @ezyang
  • removal of most patch files from the amd_build directory and integration into the code base
  • enabling of previously disabled_features that do compile now
  • improvement to the static_cast feature in pyHIPIFY (it will only apply static_cast to kernel arguments, not launch arguments)
  • addition of two workarounds to pyHIPIFY for ROCm/HIP shortcomings: a) __forceinline__ does not imply static, hence change to __inline__, b) std::[exp,log,pow] math functions cannot be selected in device code, use ::[exp,log,pow] instead. Both of these workarounds will be removed once the issues are fixed upstream. Neither of these issues have surfaced on the CI but were reproduced internally.

iotamudelta and others added 30 commits June 5, 2018 16:37
Bring our fork up to date.
Merge from upstream master into our fork.
Update the copyright dates and reflect the reality that this has
been a collaborative effort between AMD and Facebook in the last
years.
Script is now included in pytorch source.
first round of changes to update PR
This reverts commit 864dbe4.
next round of fixes to address comments
After discussion in review, disable flake8 on pyHIPIFY for now.
smem_size = block.x == 1 ? 0 : block_threads * sizeof(accscalar_t);
int max_active_blocks;
#ifdef __HIP_PLATFORM_HCC__
max_active_blocks = 16;

This comment was marked as off-topic.

This comment was marked as off-topic.

if (range > 1ULL << 32) {
generate_random_64<<<NUM_BLOCKS, BLOCK_SIZE, 0, THCState_getCurrentStream(state)>>>(
gen->state.gen_states, size, data, min_val, range);
gen->state.gen_states, static_cast<int32_t>(size), data, min_val, range);

This comment was marked as off-topic.

This comment was marked as off-topic.

PyTorch specific translations.
Each of the entries in these maps translates a CUDA string to a tuple containing the
ROCm/HIP string, a type and API annotation and - optionally - an annotation if it is not
supported in ROCm/HIP yet.

This comment was marked as off-topic.

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Looks basically reasonable.

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.

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

@ezyang
Copy link
Contributor

ezyang commented Jul 14, 2018

@iotamudelta Do you mind fixing those minor nits? Then I can merge it.

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.

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

Refactoring & Fixing the pyhipify script.
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.

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

jithunnair-amd and others added 4 commits July 16, 2018 18:22
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.

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

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.

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

zdevito pushed a commit to zdevito/ATen that referenced this pull request Jul 18, 2018
Summary:
This PR contains the ROCm contributions of last week:
* documentation of pyHIPIFY data format originating from #8812 reviewing comments by ezyang
* removal of most patch files from the `amd_build` directory and integration into the code base
* enabling of previously disabled_features that do compile now
* improvement to the static_cast feature in pyHIPIFY (it will only apply static_cast to kernel arguments, not launch arguments)
* addition of two workarounds to pyHIPIFY for ROCm/HIP shortcomings: a) `__forceinline__` does not imply `static`, hence change to `__inline__`, b) `std::[exp,log,pow]` math functions cannot be selected in device code, use `::[exp,log,pow]` instead. Both of these workarounds will be removed once the issues are fixed upstream. Neither of these issues have surfaced on the CI but were reproduced internally.
Pull Request resolved: pytorch/pytorch#9432

Differential Revision: D8887441

Pulled By: ezyang

fbshipit-source-id: 71cf5c6b13772a66d10be369a45ebf06e4e268e1
jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 30, 2018
Summary:
This PR contains the ROCm contributions of last week:
* documentation of pyHIPIFY data format originating from pytorch#8812 reviewing comments by ezyang
* removal of most patch files from the `amd_build` directory and integration into the code base
* enabling of previously disabled_features that do compile now
* improvement to the static_cast feature in pyHIPIFY (it will only apply static_cast to kernel arguments, not launch arguments)
* addition of two workarounds to pyHIPIFY for ROCm/HIP shortcomings: a) `__forceinline__` does not imply `static`, hence change to `__inline__`, b) `std::[exp,log,pow]` math functions cannot be selected in device code, use `::[exp,log,pow]` instead. Both of these workarounds will be removed once the issues are fixed upstream. Neither of these issues have surfaced on the CI but were reproduced internally.
Pull Request resolved: pytorch#9432

Differential Revision: D8887441

Pulled By: ezyang

fbshipit-source-id: 71cf5c6b13772a66d10be369a45ebf06e4e268e1
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Summary:
This PR contains the ROCm contributions of last week:
* documentation of pyHIPIFY data format originating from pytorch#8812 reviewing comments by ezyang
* removal of most patch files from the `amd_build` directory and integration into the code base
* enabling of previously disabled_features that do compile now
* improvement to the static_cast feature in pyHIPIFY (it will only apply static_cast to kernel arguments, not launch arguments)
* addition of two workarounds to pyHIPIFY for ROCm/HIP shortcomings: a) `__forceinline__` does not imply `static`, hence change to `__inline__`, b) `std::[exp,log,pow]` math functions cannot be selected in device code, use `::[exp,log,pow]` instead. Both of these workarounds will be removed once the issues are fixed upstream. Neither of these issues have surfaced on the CI but were reproduced internally.
Pull Request resolved: pytorch#9432

Differential Revision: D8887441

Pulled By: ezyang

fbshipit-source-id: 71cf5c6b13772a66d10be369a45ebf06e4e268e1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants