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

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 added a comment.

> I think that any test with a fdiv, that would by instcombine be converted to a fmul (like https://godbolt.org/z/P1bd73TK7) are OK to leave as regressions. We would not see the regression in practice as instcombine has already performed the conversion to a fmul.
>
> Do you know which cases that would not cover and would be left over if the fdiv was not handled?

Thanks for comments.
If we add some patterns with `fdiv`, we can avoid the regressions. For example,

  def : Pat<(f16 (fdiv (f16 (any_sint_to_fp (i32 GPR32:$Rn))), fixedpoint_f16_i32:$scale)),
            (SCVTFSWHri GPR32:$Rn, fixedpoint_f16_i32:$scale)>;
  def : Pat<(f32 (fdiv (f32 (any_sint_to_fp (i32 GPR32:$Rn))), fixedpoint_f32_i32:$scale)),
            (SCVTFSWSri GPR32:$Rn, fixedpoint_f32_i32:$scale)>;
  def : Pat<(f64 (fdiv (f64 (any_sint_to_fp (i32 GPR32:$Rn))), fixedpoint_f64_i32:$scale)),
            (SCVTFSWDri GPR32:$Rn, fixedpoint_f64_i32:$scale)>;

Let me add these patterns in update. If you do not like it, please let me know. Let me remove them.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16717
+/// floating-point conversion.
+static SDValue performFDivCombine(SDNode *N, SelectionDAG &DAG,
+                                  TargetLowering::DAGCombinerInfo &DCI,
----------------
dmgreen wrote:
> I think this can be dropped. If we want this transform for scalars it would be best to reuse the logic in DAGCombine.
Let me remove it.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:709-711
+  let EncoderMethod = "getFixedPointScaleOpValue";
+  let DecoderMethod = "DecodeFixedPointScaleImm32";
+  let ParserMatchClass = Imm1_32Operand;
----------------
dmgreen wrote:
> This can maybe be removed, as the class is only used as a ComplexPattern, not as a assembly Operand?
Let me remove it.


================
Comment at: llvm/test/CodeGen/AArch64/svtcf-fmul-fdiv-combine.ll:4
+
+define float @scvtf_f32_2(i32 %state) {
+; CHECK-LABEL: scvtf_f32_2:
----------------
dmgreen wrote:
> Can you add some fp16 variants in here too.
Let me add fp16 tests.


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

https://reviews.llvm.org/D156538