[PATCH] D156538: [AArch64] Try to combine FMUL with FDIV
Dave Green via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 14 00:21:01 PDT 2023
dmgreen added a comment.
In D156538#4579511 <https://reviews.llvm.org/D156538#4579511>, @jaykang10 wrote:
> @dmgreen As you can see, there are some regressions.
> After exact inverse for `fmul`, the `convertToInteger` sets `IsExact` to `false` in some cases.
> Maybe, we could need to keep the `fdiv` patterns too...
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?
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16717
+/// floating-point conversion.
+static SDValue performFDivCombine(SDNode *N, SelectionDAG &DAG,
+ TargetLowering::DAGCombinerInfo &DCI,
----------------
I think this can be dropped. If we want this transform for scalars it would be best to reuse the logic in DAGCombine.
================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:709-711
+ let EncoderMethod = "getFixedPointScaleOpValue";
+ let DecoderMethod = "DecodeFixedPointScaleImm32";
+ let ParserMatchClass = Imm1_32Operand;
----------------
This can maybe be removed, as the class is only used as a ComplexPattern, not as a assembly Operand?
================
Comment at: llvm/test/CodeGen/AArch64/svtcf-fmul-fdiv-combine.ll:4
+
+define float @scvtf_f32_2(i32 %state) {
+; CHECK-LABEL: scvtf_f32_2:
----------------
Can you add some fp16 variants in here too.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156538/new/
https://reviews.llvm.org/D156538
More information about the llvm-commits
mailing list