[PATCH] D110943: [GlobalISel] Combine fabs(fneg(x)) to fabs(x)

[Mirko Brkusanin via Phabricator via llvm-commits]

mbrkusanin added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h:371-376
   /// Match fabs(fabs(x)) to fabs(x).
   bool matchCombineFAbsOfFAbs(MachineInstr &MI, Register &Src);
   void applyCombineFAbsOfFAbs(MachineInstr &MI, Register &Src);
 
+  /// Transform fabs(fneg(x)) to fabs(x).
+  bool matchCombineFAbsOfFNeg(MachineInstr &MI, BuildFnTy &MatchInfo);
----------------
foad wrote:
> Would it be neater to combine these into a single "fabs of fabs-or-fneg" combine?
There are actually not that similar.
fabs(fabs(x)) to fabs(x) replaces outer abs with inner one and deletes the outer one.

I do not delete anything because fneg might have multiple uses. 
My main motivation for this combine is to help AMD's instruction selection when it comes to source modifiers. After abs is selected into a modifier it will stop and it wont pick neg which will live on and be redundant.
Even with multiple uses of fneg(x) those other uses can still be replaced by a neg src modifier (but because of the redundant use it won't be DCE'd).

Here is an interesting test that shows benefits even with multple uses of fneg(x):


```
define amdgpu_vs float @test(float %x, float %y) {
.entry:
    %negx = fneg float %x
    %absnegx = call float @llvm.fabs.f32(float %negx)
    %add1 = fadd float %absnegx, %y
    %add2 = fadd float %add1, %negx
    ret float %add2
}

declare float @llvm.fabs.f32(float)
```

Before patch:
```
	v_xor_b32_e32 v2, 0x80000000, v0
	v_add_f32_e64 v1, |v2|, v1
	v_add_f32_e64 v0, v1, -v0
```
After patch:
```
	v_add_f32_e64 v1, |v0|, v1
	v_add_f32_e64 v0, v1, -v0
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110943/new/

https://reviews.llvm.org/D110943