[PATCH] D154805: [DAGCombiner] Fold IEEE `fmul`/`fdiv` by Pow2 to `add`/`sub` of exp

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Can you add a test where the original fmul would fold into an fma? I think you're worse off doing this if you're interfering with FMA formation



================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3971
+  //     -> (bitcast_to_FP (add (bitcast_to_INT C), Log2(Pow2) << mantissa))
+  // (fdiv C, (uitofp Pow2))
+  //     -> (bitcast_to_FP (sub (bitcast_to_INT C), Log2(Pow2) << mantissa))
----------------
I think this should just use the fmul case, and turn the compatible fdivs to fmul 


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16188
+  if (ISD::matchUnaryPredicate(Op, IsPowerOfTwo)) {
+    if (AllSame || !VT.isVector())
+      return DAG.getConstant(Pow2Constants.back().logBase2(), DL, VT);
----------------
You don't need to bother special casing the splat case. getConstant on a vector will just split this out to a build_vector of constants anyway


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16201
+
+  auto CastToVT = [&](EVT NewVT, SDValue ToCast) {
+    EVT CurVT = ToCast.getValueType();
----------------
I'd kind of expect there to be a helper that does this already?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16216
+    // 1 << Y and X nuw/nsw << Y are all non-zero.
+    if (AssumeNonZero || isOneConstant(Op.getOperand(0)) ||
+        Op->getFlags().hasNoUnsignedWrap() || Op->getFlags().hasNoSignedWrap())
----------------
Move isOneConstant as the last condition


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16488
 
+  if (SDValue R = combineFMulOrFDivWithIntPow2(this, this->TLI, N))
+    return R;
----------------
Can you add a test where this w


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154805