Skip to content

refactor(rocrand): replace hip vector accessor macro and use underlying native vector - #203

Closed
Naraenda wants to merge 10000 commits into
ROCm:release-staging/rocm-rel-7.0from
StreamHPC:fix-rocrand-hip-vector-accessor
Closed

Naraenda wants to merge 10000 commits into
ROCm:release-staging/rocm-rel-7.0from
StreamHPC:fix-rocrand-hip-vector-accessor

Conversation

@Naraenda

Copy link
Copy Markdown
Member

Original change by @NB4444, I'm porting this over from our internal repository. This changes the original fix by @umfranzw s.t. it uses get_native_vector() instead of the newer operator[] accessor. get_native_vector() would result in better generated code since operator[] may spill to memory (see ROCm/clr#167).

@danielsu-amd

This comment was marked as off-topic.

@azure-pipelines

This comment was marked as off-topic.

@danielsu-amd

This comment was marked as off-topic.

@azure-pipelines

This comment was marked as off-topic.

@danielsu-amd

ghost commented Jun 13, 2025

Copy link
Copy Markdown

/azp run rocRAND

@azure-pipelines

ghost commented Jun 13, 2025

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@umfranzw

ghost commented Jun 17, 2025

Copy link
Copy Markdown
Contributor

This looks good, thanks! I've rerun the CI that was failing on almalinux8, since the error looked like it might be spurious.

@danielsu-amd

ghost commented Jun 17, 2025

Copy link
Copy Markdown

Hi @Naraenda @umfranzw, the almalinux8 failures were a CI issue and should now be fixed. To pick up new CI changes you do need to start a new job, rerunning the existing job will continue to use the old CI version. You can request new builds for PRs by commenting /azp run <pipeline-name>

@danielsu-amd

ghost commented Jun 17, 2025

Copy link
Copy Markdown

/azp run rocRAND

@azure-pipelines

ghost commented Jun 17, 2025

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

umfranzw
umfranzw previously approved these changes Jun 17, 2025

ghost left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

dependabot[bot] and others added 12 commits June 18, 2025 08:15
)

Bumps [sphinxcontrib-bibtex](https://github.com/mcmtroffaes/sphinxcontrib-bibtex) from 2.6.3 to 2.6.4.
- [Changelog](https://github.com/mcmtroffaes/sphinxcontrib-bibtex/blob/develop/CHANGELOG.rst)
- [Commits](mcmtroffaes/sphinxcontrib-bibtex@2.6.3...2.6.4)

---
updated-dependencies:
- dependency-name: sphinxcontrib-bibtex
  dependency-version: 2.6.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Backport of ROCm#190

Co-authored-by: Di Nguyen <dinguyennhu@gmail.com>
### Includes the following PRs:

- ROCm#124 
- ROCm#171

---------

Co-authored-by: NguyenNhuDi <dinguyennhu@gmail.com>
### Includes the following PRs:

- ROCm#126

Co-authored-by: Wayne Franz <wayfranz@amd.com>
* based on looking at the previous code, there was always a path to call ck_args.MakeArgPtr via an else statement.  We took that away with a constexpr if statment check on just DataInvokeParams.

* Similar issue to the first, calling ConvTensors was never guarded by a constexpr if.  Remove the guard to return to previous behaviour for non fusions.
🔁 Imported from
[ROCm/Tensile#2151](ROCm/Tensile#2151)
🧑‍💻 Originally authored by @bstefanuk

---------

Co-authored-by: Braden Stefanuk <121893577+bstefanuk@users.noreply.github.com>
Co-authored-by: assistant-librarian[bot] <assistant-librarian[bot]@users.noreply.github.com>
@Naraenda

ghost commented Jun 30, 2025

Copy link
Copy Markdown
Member Author

Github is being annoying with changing the base, so I'm just going to reopen the PR.

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.