Disable check_mul_result_is_nonnegative_and_representable for SYCL device code - #426
Conversation
…vice code Signed-off-by: Daniel Arndt <arndtd@ornl.gov>
nmm0
left a comment
There was a problem hiding this comment.
Thanks for digging into this, that's kinda of wild
| if ( a < 0 || b < 0 ) return false; | ||
| } | ||
| return a <= std::numeric_limits<T>::max() / b; | ||
| return true; |
There was a problem hiding this comment.
In the original code wasn't this return true; unreachable and hence, like an inelegant typo?
There was a problem hiding this comment.
Maybe the code was originally:
if (a <= std::numeric_limits<T>::max() / b)
return true;
// either else return false; or return false;
And the if was changed to return.
Even if we revert back to an if condition, there is a good chance that the compiler could still rewrite it as return (a <= std::numeric_limits<T>::max() / b);
| // FIXME_SYCL The code below compiles to old_llvm.umul.with.overflow.i64 | ||
| // which isn't defined in device code | ||
| #ifdef __SYCL_DEVICE_ONLY__ | ||
| return true; | ||
| #else |
There was a problem hiding this comment.
For what T did we get the error?
Could we not discriminate instead of wholesale disabling for all types?
There was a problem hiding this comment.
Maybe the expression a <= std::numeric_limits<T>::max() / b is causing the issue and leading to the intrinsic function llvm.umul.with.overflow.i64 being generated. Maybe we could at least have the following two checks (before the hard return true;) within #ifdef __SYCL_DEVICE_ONLY__:
if (b == 0 || a == 0)
return true;
if constexpr (std::is_signed_v<T>) {
if ( a < 0 || b < 0 ) return false;
}
|
We could report this as a compiler bug to SYCL. The function is annotated as |
We reported to Intel internally. |
I should have worded my comment more accurately in the context of SYCL. Thanks for your reply, @masterleinad 👍 |
check_mul_result_is_nonnegative_and_representablecompiles toold_llvm.umul.with.overflow.i64which isn't defined in SYCL device code. Errors look like