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

[Jay Foad via Phabricator via llvm-commits]

foad accepted this revision.
foad added a comment.
This revision is now accepted and ready to land.

LGTM.



================
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);
----------------
mbrkusanin wrote:
> 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
> ```
> 
OK.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2139
+  Register NegSrc;
+  if (mi_match(Src, MRI, m_GFNeg(m_Reg(NegSrc)))) {
+    MatchInfo = [=, &MI](MachineIRBuilder &B) {
----------------
Could return false early.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110943