The class `ext::oneapi::bfloat16` has too greedy and thus wrong definition of math/relative operators
- Dominant language
- LLVM
- Stars
- 1.5k
- Forks
- 854
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 137
Description
The issue was discovered when I tried to compare ext::intel::esimd::simd< bfloat16, N > with bfloat16.
Such comparison must return ext::intel::esimd::simd_mask< N > and the corresponding operator<() is defined inside esimd namespace.
The attempt to compile such code resulted into 'ambiguous call' error.
One candidate is the operator<() returning simd_mask< N > defined in `esimd` namespace and another candidate is defined in ext/oneapi/bfloat16.hpp
I can generalize the problem to any user type `T` as below:
1) bfloat16.hpp (shown below) defines math and comparison operators for user type `T` without user consent, which results into non-avoidable 'ambiguous call' errors if user want to re-define operators, e.g. `operator(const T&, const bfloat16&)`.
2) The declaration shown below does not follow C++ type-promotion-rules choosing heavier type. E.g. `(double + bfloat16)` must result into `double`, **not** `bfloat16` as defined below.
```c++
// Binary operators overloading
#define OP(type, op) \
friend type operator op(const bfloat16 &lhs, const bfloat16 &rhs) { \
return type{static_cast(lhs) op static_cast(rhs)}; \
} \
template \
friend type operator op(const bfloat16 &lhs, const T &rhs) { \
return type{static_cast(lhs) op static_cast(rhs)}; \
} \
template \
friend type operator op(const T &lhs, const bfloat16 &rhs) { \
return type{static_cast(lhs) op static_cast(rhs)}; \
}
OP(bfloat16, +)
OP(bfloat16, -)
OP(bfloat16, *)
OP(bfloat16, /)
OP(bool, ==)
OP(bool, !=)
OP(bool, <)
OP(bool, >)
OP(bool, <=)
OP(bool, >=)
#undef OP
```
In particular, this function:
```c++
template \
friend type operator op(const T &lhs, const bfloat16 &rhs) { \
return type{static_cast(lhs) op static_cast(rhs)}; \
}
```
blindly tries to convert any object 'const T &lhs' to 'float', then compare it with 'rhs', then return 'bool'.
This behavior conflicts with any alternative definitions such as 'esimd::simd' and any potential user's class T.
Users can easily want to have some class defining 'operator<()' differently, for example:
```c++
struct Ray {
float x;
float y;
};
bool operator<(const Ray &R, const bfloat16 &Length) { return sqrt(R.x*R.x + R.y*R.y) < Length; }
Ray operator+(const Ray &R, const bfloat16 &Extra) {/*some tricky code updating X and Y of the new vector making it a bit longer than 'R'*/}
```
Such code would get same 'ambiguous call' error
or an error during the attempt to 'static_cast< float >' of user's object that is done at bfloat16.hpp header.
The better and more safe approach is used for 'sycl::half' type defined inside 'sycl/half_type.hpp':
https://github.com/intel/llvm/blob/sycl/sycl/include/sycl/half_type.hpp#L449-L524
- OS: Windows and Linux
- Target device and vendor: Intel GPU, all other devices that support ext::oneapi::bfloat16 type
Contributor guide
Assessment
This issue has not been assessed yet.