Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

Vector types operators taking a second arbitrary argument are too greedy #4

Open
Oblomov opened this issue Oct 10, 2021 · 0 comments
Open

Comments

@Oblomov
Copy link

Oblomov commented Oct 10, 2021

This is again an issue that came up trying to introduce a HIP backend support in GPUSPH. We have Point and Vector classes that can be constructed from vector types, equipped with operators that allow adding (etc) them together.

This means that code such as this: double3 v ; Point pt ; Point n = v + pt works because the v gets converted to a Point and Point+Point is a defined operation —at least in CUDA and on CPU.

This is however does not work when using HIP vector types, because of definitions such as:

    template<typename T, unsigned int n, typename U>
    __HOST_DEVICE__
    inline
    constexpr
    HIP_vector_type<T, n> operator+(
        const HIP_vector_type<T, n>& x, U y) noexcept
    {
        return HIP_vector_type<T, n>{x} += HIP_vector_type<T, n>{y};
    }

or the one with the flipped arguments. These take precedence of the conversion-enabled Point+Point operator, and result in an error because HIP_vector_type<T, n> cannot be constructed from a Point.

The solution is to enable these operators only for types for which the conversion is defined, for example using appropriate enable_if fencing:

    template<typename T, unsigned int n, typename U>
    __HOST_DEVICE__
    inline
    constexpr
    typename std::enable_if<std::is_convertible<U, HIP_vector_type<T, n>>::value, HIP_vector_type<T, n>>::type
    operator+(
        const HIP_vector_type<T, n>& x, U y) noexcept
    {
        return HIP_vector_type<T, n>{x} += HIP_vector_type<T, n>{y};
    }

This should be done for all these operators. Additionally, the U argument should be a const& to avoid unnecessary invocations of copy constructors.

Oblomov added a commit to Oblomov/hipamd that referenced this issue Dec 16, 2021
This is necessary to avoid errors about ambiguous overloads in code such as
````

struct Point {
	float4 pos;
	float mass;
};

template<typename T>
Point operator+(Point const& p, T const& v)
{
	return Point{p.pos + v, p.mass};
}

int main() {
	float4 v = make_float4(0, 1, 2, 3);
	Point p{make_float4(3, 2, 1, 0), 1.0f};

	Point q = p + v;
}
````
when building with the host compiler.

Closes hipamd issue ROCm#4
@Oblomov Oblomov mentioned this issue Dec 16, 2021
Oblomov added a commit to Oblomov/hipamd that referenced this issue Jul 2, 2022
This is necessary to avoid errors about ambiguous overloads in code such as
````

struct Point {
	float4 pos;
	float mass;
};

template<typename T>
Point operator+(Point const& p, T const& v)
{
	return Point{p.pos + v, p.mass};
}

int main() {
	float4 v = make_float4(0, 1, 2, 3);
	Point p{make_float4(3, 2, 1, 0), 1.0f};

	Point q = p + v;
}
````
when building with the host compiler.

Closes hipamd issue ROCm#4
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant