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

Missing const T& conversion for Scalar_accessor #2359

Closed
Oblomov opened this issue Sep 21, 2021 · 3 comments
Closed

Missing const T& conversion for Scalar_accessor #2359

Oblomov opened this issue Sep 21, 2021 · 3 comments

Comments

@Oblomov
Copy link

Oblomov commented Sep 21, 2021

Hello,

I'm in the process of adding HIP support to GPUSPH, and one of the obstacles I've encountered so far is the lack of a operator const T&() const conversion operator for Scalar_accessor. The issue manifests only when building the host code using g++ $(hipconfig -C), and can be observed e.g. with this simple test case:

#include <hip/hip_runtime.h>

static __device__ __forceinline__ __host__ float clamp(const float &f, const float &a, const float &b)
{
	return fmaxf(a, fminf(f, b));
}

static __device__ __forceinline__ __host__ float2 clamp(const float2 &f, const float &a, const float &b)
{
	return make_float2( clamp(f.x, a, b), clamp(f.y, a, b) );
}

compiled with g++ $(hipconfig -C | tr -d \") -c test.cc, giving the error

test.cc: In function ‘float2 clamp(const float2&, const float&, const float&)’:
test.cc:10:30: error: invalid user-defined conversion from ‘const hip_impl::Scalar_accessor<float, float [2], 0>’ to ‘const float&’ [-fpermissive]
   10 |  return make_float2( clamp(f.x, a, b), clamp(f.y, a, b) );
      |                            ~~^
In file included from /opt/rocm-4.3.0/hip/include/hip/amd_detail/channel_descriptor.h:28,
                 from /opt/rocm-4.3.0/hip/include/hip/amd_detail/hip_texture_types.h:38,
                 from /opt/rocm-4.3.0/hip/include/hip/amd_detail/hip_runtime_api.h:40,
                 from /opt/rocm-4.3.0/hip/include/hip/hip_runtime_api.h:410,
                 from /opt/rocm-4.3.0/hip/include/hip/hip_runtime.h:113,
                 from test.cc:1:
/opt/rocm-4.3.0/hip/include/hip/amd_detail/hip_vector_types.h:141:13: note: candidate is: ‘hip_impl::Scalar_accessor< <template-parameter-1-1>, <template-parameter-1-2>, <anonymous> >::operator T&() [with T = float; Vector = float [2]; unsigned int idx = 0]’ (near match)
  141 |             operator T&() noexcept {
      |             ^~~~~~~~

The same error does not appear when compiling with hipcc, or when using clang++ instead of g++ as host compiler, so it's possible that the issue is due to the different representation used for vector types in the two cases. I noticed that the Scalar_accessor does provide operator T() const conversions, but these aren't apparently sufficient to satisfy g++. A solution could be to conver them into const T& operators, that should work just as well for both the T and const T& case.

@Oblomov
Copy link
Author

Oblomov commented Sep 21, 2021

Another example, with a simpler test-case that also fails with clang++:

#include <hip/hip_runtime.h>

typedef ushort4 particleinfo;

static __forceinline__ __host__ __device__ __attribute__((pure))
const ushort& type(const particleinfo &info)
{ return info.x; }

compiled with clang++ $(hipconfig -C) -c will give the warning:

test2.cc:7:10: warning: returning reference to local temporary object [-Wreturn-stack-address]
{ return info.x; }

Replacing the operator T() const with appropriate operator const T&() const operators fixes this issue as well.

@ppanchad-amd
Copy link

@Oblomov Apologies for the lack of response. Can you please test with latest ROCm 6.0.2 (HIP 6.0.32831)? If resolved, please close ticket. Thanks!

@Oblomov
Copy link
Author

Oblomov commented Apr 12, 2024

Thank you, this seems to be fixed in 6.0.2

@Oblomov Oblomov closed this as completed Apr 12, 2024
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

No branches or pull requests

2 participants