Skip to content
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.

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

added feature for locking host memory #24

Closed
wants to merge 3 commits into from
Closed

added feature for locking host memory #24

wants to merge 3 commits into from

Conversation

aditya4d
Copy link
Contributor

No description provided.

@@ -0,0 +1,24 @@
// XFAIL: Linux
// RUN: %hc %s -o %t.out %t.out
Copy link
Collaborator

Choose a reason for hiding this comment

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

@adityaatluri this line looks suspicious. The test is supposed to be linked with -lhc_am but it's not specified here.

@whchung
Copy link
Collaborator

whchung commented Apr 14, 2016

@adityaatluri could you help:

  1. address my comment inside the codes
  2. alter the pull request so it goes to a feature branch "mem_lock" first
  3. provide test results of "HCC_RUNTIME=HSA make test"
  4. provide test results of HIP unit tests

@@ -0,0 +1,34 @@
// XFAIL: Linux
// RUN: %hc %s -o %t.out %t.out
Copy link
Collaborator

Choose a reason for hiding this comment

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

@adityaatluri same here. perhaps -lhc_am is needed so lit could correct build the test

@whchung
Copy link
Collaborator

whchung commented Apr 14, 2016

Hi Aditya,

For point 2, normally I'd like to have new features go to a branch first.
And I'll merge them into master once I verify it again. And this pull
request wants to merge 1 commit into RadeonOpenCompute:master from
adityaatluri:mem_lock . I'd like to ask you change it so it goes to
RadeonOpenCompute:mem_lock . On bitbucket it's quite intuitive to do so,
but it seems Github UI is a bit strange here. You should be able to find
"mem_lock" when you create a pull request.

For point 3 & 4, please run "HCC_RUNTIME=HSA make test" and HIP unit tests
as a guarantee that your new check-ins don't break existing stuffs. You can
refer to this pull request as an example.

#20

Jack

2016-04-14 10:18 GMT+08:00 Aditya Atluri notifications@github.com:

Hi @whchung https://github.com/whchung ,
Except for point 1, I have no idea about the rest you are talking about. I
don't know about the unit test infrastructure of hcc. I want to add this
functionality for friday release.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#24 (comment)

if(all.size() == 0){
return -1;
}
std::vector<hsa_agent_t> agentVec;
Copy link
Collaborator

Choose a reason for hiding this comment

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

please consider not using HSA data structures directly in hcc unit tests.

if you must use them. please refer to:
https://github.com/RadeonOpenCompute/hcc/blob/master/tests/Unit/HC/create_marker.cpp
for an example. you'll basically need to change lit macro to make your test build.

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.

3 participants