Skip to content

support dynamic buffer using memory coherence glc_slc bit from template#725

Merged
zjing14 merged 4 commits into
developfrom
support_amd_buffer_glc_slc
Jun 8, 2023
Merged

support dynamic buffer using memory coherence glc_slc bit from template#725
zjing14 merged 4 commits into
developfrom
support_amd_buffer_glc_slc

Conversation

@carlushuang
Copy link
Copy Markdown
Contributor

@carlushuang carlushuang commented May 25, 2023

  • add amd_buffer_coherence_bits enum to switch between different coherence bits in buffer load/store.
  • support changing coherence while creating a DynamicBuffer through template arg (so we no need to change every threadwise copy)
  • support changing coherence with make_dynamic_buffer() function
  • The default value is amd_buffer_coherence_bits::default_coherence

The change in this P.R has been used and verified by

gridwise_multiblock_batchnorm_forward_generic.hpp

@poyenc poyenc self-requested a review May 25, 2023 09:37
qianfengz
qianfengz previously approved these changes May 26, 2023
Comment thread include/ck/utility/amd_buffer_addressing.hpp
Comment thread include/ck/utility/amd_buffer_addressing.hpp Outdated
Comment thread include/ck/utility/amd_buffer_addressing.hpp Outdated
Copy link
Copy Markdown
Contributor

@qianfengz qianfengz left a comment

Choose a reason for hiding this comment

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

The latest change works with my BatchNorm split-K testing example

Comment on lines +296 to +299
DefaultCoherence = 0, // default value
GLC = 1,
SLC = 2,
GLC_SLC = 3,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be nice to have here explanation of what behavior to expect from each enumerator value. In the mentioned document the page 58 explains also the SLC bit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @aosewski , the problem is, different architecture may have different meanings, so I still suggest we have the name only

@carlushuang carlushuang requested a review from asroy June 6, 2023 08:17
@zjing14 zjing14 merged commit 016ebaa into develop Jun 8, 2023
@illsilin illsilin deleted the support_amd_buffer_glc_slc branch December 7, 2023 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants