[PATCH] D156538: [AArch64] Try to combine FMUL with FDIV

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 11 00:48:58 PDT 2023


dmgreen added a comment.

In D156538#4576702 <https://reviews.llvm.org/D156538#4576702>, @jaykang10 wrote:

> @dmgreen As you can see, `DAGCombiner::visitFDIV` does the conversion with `Options.UnsafeFPMath || Flags.hasAllowReciprocal()` so the patterns with `fdiv` are not selected without the options.
> In order to use only `fmul`, we could need to remove the `Options.UnsafeFPMath || Flags.hasAllowReciprocal()`. Alternatively, We could need to support patterns with both `fmul` and `fdiv` with different complex patterns. How do you think about it?

There is an instcombine version here: https://github.com/llvm/llvm-project/blob/c2093b85044d87805c39267c65ac9032d5454e0e/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp#L1546. It seems to do the transform whenever the value has an exact inverse. `// If the constant divisor has an exact inverse, this is always safe.`  Alive doesn't run to completion to prove it (https://alive2.llvm.org/ce/z/mxheqK), but doesn't come up with a failure in that time. Can we use the same logic to always to the transform? I don't know of any counter examples where it wouldn't be true.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:3680
 
+bool AArch64DAGToDAGISel::SelectCVTFixedPosRecipOperand(SDValue N,
+                                                        SDValue &FixedPos,
----------------
Could this be shared with SelectCVTFixedPosOperand? Maybe with a flag to specify whether the getExactInverse needs to be performed. If not them maybe the ConstantFPSDNode/ConstantPoolSDNode stuff can be pulled out and shared?


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

https://reviews.llvm.org/D156538



More information about the llvm-commits mailing list