Skip to content

EXSWHTEC-109 - Implement tests for the hipModuleLoad family of APIs and hipModuleUnload - #20

Closed
music-dino wants to merge 22 commits into
ROCm:developfrom
mirza-halilcevic:hipModuleLoad_tests
Closed

EXSWHTEC-109 - Implement tests for the hipModuleLoad family of APIs and hipModuleUnload#20
music-dino wants to merge 22 commits into
ROCm:developfrom
mirza-halilcevic:hipModuleLoad_tests

Conversation

@music-dino

@music-dino music-dino commented Dec 7, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Implement positive and negative tests for hipModuleLoad
  • Implement positive and negative tests for hipModuleLoadData
  • Implement positive and negative tests for hipModuleLoadDataEx
  • Implement positive tests for hipModuleUnload as part of the above tests
  • Implement negative tests for hipModuleUnload.

gargrahul and others added 9 commits October 26, 2022 03:59
@gargrahul
SWDEV-355313 - Move catch tests and samples
cea96af 
Change-Id: I66f0c09e9c7405ec7430b1883e0e89542fdb87a0
@gargrahul
SWDEV-355313 - Add README
909e7e4 
Change-Id: I212b82b1b3a78a368b85ea64e338371a34b405f9
@gargrahul
SWDEV-355313 - Update amd-staging branch
094b9af 
Change-Id: Ib455f72b5be77e1a81137d15c07ea41161b16a3e
@gargrahul
SWDEV-355313 - Update README
9daa6d0 
Change-Id: Ief96e274f4143e80ceb3e40f04d38ae217777583
@gargrahul
SWDEV-355313 - Update latest code
c49043e 
Change-Id: I9c03cde09b42c8e3726153c2a177359efc8d6d29
@music-dino
Create common code for module tests
7a213be
@music-dino
Create common code for module tests
580df9d
@music-dino
Merge branch 'module_common' into hipModuleLoad_tests
f40fe6a
@music-dino
EXSWHTEC-109 - Implement tests for the hipModuleLoad family of APIs a…
afa2e93 
…nd hipModuleUnload

- Basic positive tests
- Negative parameter tests
@milos-mozetic milos-mozetic mentioned this pull request Dec 19, 2022
Open
mangupta
mangupta requested changes Mar 3, 2023
View reviewed changes
Comment thread catch/unit/module/CMakeLists.txt Outdated
Comment on lines +11 to +13
add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/empty_module.code
COMMAND ${CMAKE_CXX_COMPILER} --genco --std=c++17 ${CMAKE_CURRENT_SOURCE_DIR}/empty_module.cc -o empty_module.code
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/empty_module.cc)

@mangupta mangupta Mar 3, 2023

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.

genco does not work properly on windows yet. you will need to exclude this test building on windows. additionally you are missing specifying the offload arch & output path for building the code object. see unit/deviceLib/CMakeLists.txt for an example of how to do it for all of the above.

@music-dino music-dino Mar 6, 2023

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.

Requested changes have been implemented, please proceed.

chrispaquot
chrispaquot reviewed Apr 14, 2023
View reviewed changes
Comment thread catch/unit/module/hipModuleLoad.cc Outdated
}

// Disabled for AMD due to defect - EXSWHTEC-151
#if HT_NVIDIA

@chrispaquot chrispaquot Apr 14, 2023

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.

Create a new TEST_CAST for this and disable it via json.

@music-dino music-dino Apr 18, 2023

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.

The requested change has been implemented.

Comment thread catch/unit/module/hipModuleLoadData.cc Outdated
}

// Disabled for AMD due to defect - EXSWHTEC-153
#if HT_NVIDIA

@chrispaquot chrispaquot Apr 14, 2023

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.

Same here.

@music-dino music-dino Apr 18, 2023

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.

The requested change has been implemented.

Comment thread catch/unit/module/hipModuleLoadDataEx.cc Outdated
}

// Disabled for AMD due to defect - EXSWHTEC-153
#if HT_NVIDIA

@chrispaquot chrispaquot Apr 14, 2023

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.

again

@music-dino music-dino Apr 18, 2023

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.

The requested change has been implemented.

Comment thread catch/unit/module/hipModuleUnload.cc Outdated
HIP_CHECK(hipFree(nullptr));

// Disabled for AMD due to defect - EXSWHTEC-152
#if HT_NVIDIA

@chrispaquot chrispaquot Apr 14, 2023

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.

again

@music-dino music-dino Apr 18, 2023

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.

The requested change has been implemented.

Comment thread catch/unit/module/not_a_module.txt

@chrispaquot chrispaquot Apr 14, 2023

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.

is the empty file necessary?

@music-dino music-dino Apr 18, 2023

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.

The test requires a file that exists in the filesystem, but does not represent a valid module file. This seemed like the most straightforward approach.

@music-dino

music-dino commented Jun 26, 2023

Copy link
Copy Markdown
Contributor Author

@rorake I don't see your comment for some reason, so I cannot quote reply to it. Merge conflicts have been resolved, please proceed.

@rakesroy

rakesroy commented Feb 26, 2024

Copy link
Copy Markdown
Contributor

PR has been merged into develop branch via commit 2c6d940.

@rakesroy rakesroy closed this Feb 26, 2024
rocm-ci pushed a commit that referenced this pull request Feb 26, 2024
@mirza-halilcevic @rakesroy
EXSWHTEC-109 - Implement tests for the hipModuleLoad family of APIs a…
2c6d940 
…nd hipModuleUnload #20

Change-Id: I19d6534af2c33046dc3862372d45f32b6ccc6ad1
rocm-ci pushed a commit that referenced this pull request Feb 26, 2024
@rakesroy
SWDEV-1 - Merge github PRs to amd-staging
96df1fd 
- #119
- #151
- #57
- #58
- #59
- #60
- #99
- #139
- #152
- #48
- #54
- #53
- #24
- #23
- #22
- #21
- #20
- #14
- #8

Change-Id: I1eea54cd1436f3ddbfd5c1b3b2f672eb81d03cd4
jayhawk-commits pushed a commit to ROCm/rocm-systems that referenced this pull request Aug 18, 2025
@rakesroy
SWDEV-1 - Merge github PRs to amd-staging
fa871e2 
- ROCm/hip-tests#119
- ROCm/hip-tests#151
- ROCm/hip-tests#57
- ROCm/hip-tests#58
- ROCm/hip-tests#59
- ROCm/hip-tests#60
- ROCm/hip-tests#99
- ROCm/hip-tests#139
- ROCm/hip-tests#152
- ROCm/hip-tests#48
- ROCm/hip-tests#54
- ROCm/hip-tests#53
- ROCm/hip-tests#24
- ROCm/hip-tests#23
- ROCm/hip-tests#22
- ROCm/hip-tests#21
- ROCm/hip-tests#20
- ROCm/hip-tests#14
- ROCm/hip-tests#8

Change-Id: I1eea54cd1436f3ddbfd5c1b3b2f672eb81d03cd4


[ROCm/hip-tests commit: 96df1fd]
ammallya pushed a commit that referenced this pull request Oct 28, 2025
@mirza-halilcevic @rakesroy
EXSWHTEC-109 - Implement tests for the hipModuleLoad family of APIs a…
f1d92a5 
…nd hipModuleUnload #20

Change-Id: I19d6534af2c33046dc3862372d45f32b6ccc6ad1


[ROCm/hip-tests commit: 2c6d940]
ammallya pushed a commit that referenced this pull request Oct 28, 2025
@rakesroy
SWDEV-1 - Merge github PRs to amd-staging
abf09e1 
- #119
- #151
- #57
- #58
- #59
- #60
- #99
- #139
- #152
- #48
- #54
- #53
- #24
- #23
- #22
- #21
- #20
- #14
- #8

Change-Id: I1eea54cd1436f3ddbfd5c1b3b2f672eb81d03cd4


[ROCm/hip-tests commit: 96df1fd]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrispaquot chrispaquot chrispaquot approved these changes
@mangupta mangupta mangupta requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@music-dino @rakesroy @mangupta @chrispaquot @gargrahul @milos-mozetic @mirza-halilcevic