Skip to content

GEMMTestSuite: perform input data generation on GPU (incl. hiprand) #417

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
Open
matthiasdiener wants to merge 18 commits into
base: dev
Choose a base branch
from
Open

GEMMTestSuite: perform input data generation on GPU (incl. hiprand) #417

matthiasdiener wants to merge 18 commits into from

Conversation

@matthiasdiener
Copy link
Contributor

@matthiasdiener matthiasdiener commented Jan 16, 2026 •
edited
Loading

Description

Use hiprand random number generation (instead of CPU/OpenMP).

Partly addresses https://github.com/ROCm/frameworks-internal/issues/14746.

Notes:

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Use hiprand random number generation (instead of CPU/OpenMP).

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@matthiasdiener matthiasdiener self-assigned this Jan 16, 2026
@matthiasdiener matthiasdiener marked this pull request as ready for review January 16, 2026 18:37
alextmagro
alextmagro reviewed Jan 16, 2026
View reviewed changes
Copy link
Contributor

@alextmagro alextmagro left a comment

Choose a reason for hiding this comment

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

Awesome work! Do you have any rough estimates of how much faster the entire cpp test suite is?

@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented Jan 16, 2026

Awesome work! Do you have any rough estimates of how much faster the entire cpp test suite is?

Thanks! I left some performance numbers here: https://github.com/ROCm/frameworks-internal/issues/14746#issuecomment-3761360289

@ipanfilo
Copy link
Collaborator

ipanfilo commented Jan 17, 2026

Update copyright date of modified files

ipanfilo
ipanfilo reviewed Jan 17, 2026
View reviewed changes
tests/cpp/test_common.cu Outdated
TRANSFORMER_ENGINE_TYPE_SWITCH_ALL(t->dtype(), T,
{
#ifdef __HIP_PLATFORM_AMD__
fillUniformDevice(t);
Copy link
Collaborator

@ipanfilo ipanfilo Jan 17, 2026

Choose a reason for hiding this comment

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

Is there any test that tests this generation? I think using GPU generation here does not produce correct result because of using t->from_cpu() below in this method

Copy link
Contributor Author

@matthiasdiener matthiasdiener Jan 21, 2026

Choose a reason for hiding this comment

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

Good catch, thanks. I disabled the from_cpu call here, and added an explicit copy of the data to CPU in 0f008e9, as there are other places that copy from CPU->GPU (like set_scale_inv). I also added a new test to check that GPU and CPU copies are the same in 097ecd4.

tests/cpp/test_common.cu Outdated
rocrand_generate_uniform(gen, tmp, N);

// map to [-2.0, 1.0] (like generate_data_uniformly) and cast into tensor dtype
TRANSFORMER_ENGINE_TYPE_SWITCH_ALL(t->dtype(), T, {
Copy link
Collaborator

@ipanfilo ipanfilo Jan 17, 2026

Choose a reason for hiding this comment

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

T should either be template parameter and no TRANSFORMER_ENGINE_TYPE_SWITCH_ALL here, or the method calling should be moved out of TRANSFORMER_ENGINE_TYPE_SWITCH_ALL in fillUniform

Copy link
Contributor Author

@matthiasdiener matthiasdiener Jan 21, 2026

Choose a reason for hiding this comment

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

With the restructuring in bdb8349, I believe this comment is now addressed?

Copy link
Collaborator

@ipanfilo ipanfilo Jan 30, 2026

Choose a reason for hiding this comment

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

Seems not. fillUniformTensorDevice is still called from t->dtype() switch and in turn calls fillUniformLinearBuferDevice from t->dtype() switch

Copy link
Contributor Author

@matthiasdiener matthiasdiener Jan 30, 2026

Choose a reason for hiding this comment

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

You're right. What do you think of 33f6124?

wangye805
wangye805 requested changes Jan 20, 2026
View reviewed changes
@matthiasdiener matthiasdiener changed the title GEMMTestSuite: use rocrand for input data generation GEMMTestSuite: perform input data generation on GPU (incl. rocrand) Jan 21, 2026
@matthiasdiener matthiasdiener force-pushed the gemmtestsuite-rocrand branch 2 times, most recently from cfb4b0d to cff44ef Compare January 21, 2026 20:24
@matthiasdiener matthiasdiener changed the title GEMMTestSuite: perform input data generation on GPU (incl. rocrand) GEMMTestSuite: perform input data generation on GPU (incl. hiprand) Jan 21, 2026
alextmagro
alextmagro approved these changes Jan 21, 2026
View reviewed changes
Copy link
Contributor

@alextmagro alextmagro left a comment

Choose a reason for hiding this comment

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

LGTM! A few small comments, but otherwise excited to see the benefits for our CI.

wangye805
wangye805 requested changes Jan 22, 2026
View reviewed changes
wangye805
wangye805 approved these changes Jan 22, 2026
View reviewed changes
Copy link
Collaborator

@wangye805 wangye805 left a comment

Choose a reason for hiding this comment

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

LGTM

ipanfilo
ipanfilo requested changes Jan 30, 2026
View reviewed changes
tests/cpp/test_common.cu Outdated
rocrand_generate_uniform(gen, tmp, N);

// map to [-2.0, 1.0] (like generate_data_uniformly) and cast into tensor dtype
TRANSFORMER_ENGINE_TYPE_SWITCH_ALL(t->dtype(), T, {
Copy link
Collaborator

@ipanfilo ipanfilo Jan 30, 2026

Choose a reason for hiding this comment

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

Seems not. fillUniformTensorDevice is still called from t->dtype() switch and in turn calls fillUniformLinearBuferDevice from t->dtype() switch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alextmagro alextmagro alextmagro approved these changes

@ipanfilo ipanfilo ipanfilo requested changes

@wangye805 wangye805 wangye805 approved these changes

@wenchenvincent wenchenvincent Awaiting requested review from wenchenvincent wenchenvincent is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@matthiasdiener matthiasdiener

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@matthiasdiener @ipanfilo @wangye805 @alextmagro