llvm / llvm/llvm-project

[hexagon] unordered floating point comparisons `<=` and `>=` produce wrong results

Open
#220,983 1 comment 0 reactions 1 assignee Assigned to @iajbar View on GitHub
backend:Hexagon
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.