[InstCombine] miss fabs(sqrt(x)) > C fold for positive C
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
InstCombine does not remove a redundant `fabs` in an ordered comparison against a positive constant when the operand is the result of `sqrt`.
Reduced IR:
```llvm
define i1 @miss(double %x) {
entry:
%s = call double @llvm.sqrt.f64(double %x)
%f = call double @llvm.fabs.f64(double %s)
%c = fcmp ogt double %f, 1.000000e+00
ret i1 %c
}
```
Current behavior (https://godbolt.org/z/WxocWq3jT) keeps the fabs.
Expected:
```llvm ir
define i1 @miss(double %x) {
entry:
%s = call double @llvm.sqrt.f64(double %x)
%c = fcmp ogt double %s, 1.000000e+00
ret i1 %c
}
```
`sqrt()` has three possible FP classes: NaN, -0.0 or +0.0, and positive. All of them arithmetically ignore `fabs()` within a `fcmp` with a positive C.
Online alive2 timed out, so I paste my local running result here:
This is a real pattern observed from SPEC 2017 `511.povray_r`. The original context is like:
```c
inline void VLength(DBL& a, const VECTOR b)
{
a = sqrt(b[X] * b[X] + b[Y] * b[Y] + b[Z] * b[Z]);
}
static DBL normalize(VECTOR A, VECTOR B)
{
DBL tmp;
VLength(tmp, B);
if (fabs(tmp) > EPSILON) // redundant fabs()
{
VInverseScale(A, B, tmp);
}
else
{
Make_Vector(A, 0.0, 1.0, 0.0);
}
return(tmp);
}
```
In its final AArch64 assembly there is exactly the redundant fabs:
```assembly
fsqrt d2, d2
fabs d3, d2
fcmp d3, d15
```
Maybe on `fabs()` we could explicitly query FPClass to remove it based on the data flow facts, rather than relying on a specific pattern.
Contributor guide
Assessment
This issue has not been assessed yet.