[PATCH] D73588: AMDGPU: Enhancement on FDIV lowering in AMDGPUCodeGenPrepare

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:611
 //
-// 1/x -> rcp(x) when fast unsafe rcp is legal or fpmath >= 2.5ULP with
-//                                                denormals flushed.
+// 1/x -> rcp(x) when fdiv is allowed to be re-associated or rcp is accurate.
 //
----------------
cfang wrote:
> arsenm wrote:
> > This has nothing to do with reassociation
> Division re-association:  a/b -> a * rcp(b), and one special case is 1.0/b => 1.0*rcp(b) = rcp(b).
> This is how 1.0/x -> rcp(x) associated with "re-association".  
This isn't reassocation. This is just special handling of 1.0/b. Nothing algebraic changes here. There's no multiply introduced here


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:614
+// a/b -> a*rcp(b) when fdiv is allowed to be re-associated.
+static Value *lowerUsingRcp (Value *Num, Value *Den, bool CanReassociateFDiv,
+                             bool RcpIsAccurate, IRBuilder<> Builder,
----------------
cfang wrote:
> arsenm wrote:
> > This should not be referred to ass lowering
> I am thinking of a different name.  Do you have a meaningful name for the function in mind?
combineRcp?


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

https://reviews.llvm.org/D73588