[hexagon] unordered floating point comparisons `<=` and `>=` produce wrong results
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
According to the documentation "v81 HVX Programmer Reference Manual.html" chapter "Compare vectors"
> IEEE single precision and half precision cmp instructions return false if at least one input is a NaN.
The ISA only supports `cmp.eq` and `cmp.gt` so it seems that the compiler produces code equivalent to
```c++
a == b --> a == b
a != b --> !(a == b)
a < b --> b > a
a <= b --> !(a > b)
a > b --> a > b
a >= b --> !(b > a)
```
But for `<=` and `>=` if one operand is NaN the result is wrongly `true`. In both cases before negation `>` the comparisons returns `false` in case of NaN but the negation `!` produces the wrong result `true`.
(note implementing `!=` in terms of `!(==)` is okay because NaN `!=` NaN is `true`)
An example can be seen here:
https://godbolt.org/z/ocM1f5ndo
```c++
less_equal(float*, float*, unsigned int*):
{
r3 = #1
v30 = vxor(v30,v30)
v0 = vmem(r0+#0)
allocframe(r29,#0):raw
}
{
v31 = vsplat(r3)
q0 = vcmp.gt(v0.sf,v1.sf)
v1.cur = vmem(r1+#0)
}
{
v0 = vmux(q0,v30,v31)
vmem(r2+#0) = v0.new
}
{
r31:30 = dealloc_return(r30):raw
}
```
If `v0` or `v1` is NaN `q0 = vcmp.gt(v0.sf,v1.sf)` returns `false`. The instruction `vmux(q0,v30,v31)` basically negates the result: if `q0` is true it selects `v30` (0, false)--otherwise `v31` (1, true). Thus producing the wrong result `true` in case of NaN.
I think this should be fixed because luckily floating comparisons now with v81 correctly handle NaNs but the compiler turns it into the wrong result and all other comparisons `==`, `!=`, `<` and `>` yield the correct result.
Possible solutions could be:
less_equal with just `eq` and `gt` could be implement as `Q6_Q_vcmp_eqor_QVsfVsf(Q6_Q_vcmp_gt_VsfVsf(b, a), a, b);`
(In my point of view the ISA is missing a greater_equal instruction. Then all comparisons could be implemented with just 1 instruction)
Contributor guide
Assessment
This issue has not been assessed yet.