[PATCH] D50714: [InstCombine] Fold Select with binary op - FP opcodes

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:85
+  // Check for signed zeros
+  if (isa<FPMathOperator>(BO) && !BO->hasNoSignedZeros())
+    return nullptr;
----------------
spatel wrote:
> I think this is a correct predicate, but stronger than necessary. Does alive-fp say we can get away with CannotBeNegativeZero(Y)? If it doesn't recognize that function, you could fake it by adding another operation that creates 'Y' (or '%z' as shown in the test names) while guaranteeing that it is not -0.0. So something like:
> %z = fadd %v, 0.0 (this can never produce -0.0)
```
%A = fcmp oeq float %x, 0.0
%z = fadd %v, 0.0
%B = fadd nsz float %x, %z
%C = select i1 %A, float %B, float %y
=>
%C = select i1 %A, float %z, float %y
----------                                                                      
  %A = fcmp oeq float %x, 0.0
  %z = fadd %v, 0.0
  %B = fadd nsz float %x, %z
  %C = select %A, float %B, float %y
=>
  %C = select %A, float %z, float %y

Done: 1                                                                         
Optimization is correct
```

```
%A = fcmp oeq float %x, 0.0
%z = fadd %v, 0.0
%B = fadd float %x, %z
%C = select i1 %A, float %B, float %y
=>
%C = select i1 %A, float %z, float %y
----------                                                                      
  %A = fcmp oeq float %x, 0.0
  %z = fadd %v, 0.0
  %B = fadd float %x, %z
  %C = select %A, float %B, float %y
=>
  %C = select %A, float %z, float %y

Done: 1                                                                         
Optimization is correct

```




================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:85
+  // Check for signed zeros
+  if (isa<FPMathOperator>(BO) && !BO->hasNoSignedZeros())
+    return nullptr;
----------------
xbolva00 wrote:
> spatel wrote:
> > I think this is a correct predicate, but stronger than necessary. Does alive-fp say we can get away with CannotBeNegativeZero(Y)? If it doesn't recognize that function, you could fake it by adding another operation that creates 'Y' (or '%z' as shown in the test names) while guaranteeing that it is not -0.0. So something like:
> > %z = fadd %v, 0.0 (this can never produce -0.0)
> ```
> %A = fcmp oeq float %x, 0.0
> %z = fadd %v, 0.0
> %B = fadd nsz float %x, %z
> %C = select i1 %A, float %B, float %y
> =>
> %C = select i1 %A, float %z, float %y
> ----------                                                                      
>   %A = fcmp oeq float %x, 0.0
>   %z = fadd %v, 0.0
>   %B = fadd nsz float %x, %z
>   %C = select %A, float %B, float %y
> =>
>   %C = select %A, float %z, float %y
> 
> Done: 1                                                                         
> Optimization is correct
> ```
> 
> ```
> %A = fcmp oeq float %x, 0.0
> %z = fadd %v, 0.0
> %B = fadd float %x, %z
> %C = select i1 %A, float %B, float %y
> =>
> %C = select i1 %A, float %z, float %y
> ----------                                                                      
>   %A = fcmp oeq float %x, 0.0
>   %z = fadd %v, 0.0
>   %B = fadd float %x, %z
>   %C = select %A, float %B, float %y
> =>
>   %C = select %A, float %z, float %y
> 
> Done: 1                                                                         
> Optimization is correct
> 
> ```
> 
> 
So we can use 

ValueTracker::CannotBeNegativeZero ?


https://reviews.llvm.org/D50714