-
-
Notifications
You must be signed in to change notification settings - Fork 428
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
Fix errors coming from hpx compute examples #2415
Conversation
@@ -60,7 +60,8 @@ int hpx_main(boost::program_options::variables_map& vm) | |||
int i = blockDim.x * blockIdx.x + threadIdx.x; | |||
if(i < N) | |||
C[i] = A[i] + B[i]; | |||
}, d_A.data(), d_B.data(), d_C.data()); | |||
}, (int*)(d_A.data()), (int*)(d_B.data()), (int*)(d_C.data()) | |||
); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change really necessary? Or is it a left-over?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvcc is able to cast those target_ptr but not clang. When I remove the (int*)(...)
, it says:
note: candidate function not viable: no known conversion from 'hpx::compute::cuda::target_ptr<int>' to 'int *' for 1st argument
[=] __device__ (int* A, int* B, int* C) mutable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this cast is clearly wrong as it happens in host code where d_A.data()
returns a target_ptr<int>
and not an int*
. I rather think this is a bug and should be reported to the clang guys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casts cancelled in a89f941
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem I'm referring to is that clang apparently instantiates this function in host code, which leads to the compilation error you're trying to work around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hkaiser Is it possible for you to submit a reduced testcase to the Clang guys?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it from the clang documentation :
clang uses merged parsing. This is similar to split compilation, except all of the host and device code is present and must be semantically-correct in both compilation steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So they will probably not fix it :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This patch fixes few errors that come from hpx compute examples. The partitioned_vector example still fails at compilation.