[GISel] Add identity and fneg combines for G_FMUL

[mkitzan]

Summary

Patch adds two simple combines:

• G_FMUL(x, 1.0) -> x
• G_FMUL(G_FNEG(x), G_FNEG(y)) -> G_FMUL(x, y)

Patch additionally adds new combine tests for AArch64 target for these new rules.
(Split off from https://reviews.llvm.org/D87870)

[foad]

General question about all these combines: why do we need them? Why wouldn't they be done at the IR level, or are they cleaning up code that could reasonably be introduced by legalization?

[mkitzan]

These are combines which seem generally beneficial (not architecture specific) from the closed source backend I work on. They are beneficial for our backend, so we have been upstreaming these combines to improve GISel.

[arsenm]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

2434–2435

This is illegal, no reason to check for it. Really we should just consistently use Register in the API

[arsenm]

Can you try using new tablegen combiner support?

llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h

public:
   /// Return true if \p MOP1 and \p MOP2 are register operands are defined by
   /// equivalent instructions.
   bool matchEqualDefs(const MachineOperand &MOP1, const MachineOperand &MOP2);
 
   /// Return true if \p MOP is defined by a G_CONSTANT with a value equal to
   /// \p C.
   bool matchConstantOp(const MachineOperand &MOP, int64_t C);
 
+  /// Return true if \p MOP is defined by a G_FCONSTANT with a value equal to
+  /// \p C.
+  bool matchFConstantOp(const MachineOperand &MOP, double C);
+
   /// Optimize (cond ? x : x) -> x
   bool matchSelectSameVal(MachineInstr &MI);
 
   /// Optimize (x op x) -> x
   bool matchBinOpSameVal(MachineInstr &MI);
 
   /// Check if operand \p OpIdx is zero.
   bool matchOperandIsZero(MachineInstr &MI, unsigned OpIdx);
public:
   bool matchCombineFSubFpExtFNegFMulToFMadOrFMA(MachineInstr &MI,
                                                 BuildFnTy &MatchInfo);
 
   /// Fold boolean selects to logical operations.
   bool matchSelectToLogical(MachineInstr &MI, BuildFnTy &MatchInfo);
 
   bool matchCombineFMinMaxNaN(MachineInstr &MI, unsigned &Info);
 
+  /// Transform G_FMUL(G_FNEG(x), G_FNEG(y)) to G_FMUL(x, y).
+  bool matchFMulWithNegatedInputs(MachineInstr &MI,
+                                  std::pair<Register, Register> &MatchInfo);
+  bool applyFMulWithNegatedInputs(MachineInstr &MI,
+                                  std::pair<Register, Register> &MatchInfo);
+
 private:
   /// Given a non-indexed load or store instruction \p MI, find an offset that
   /// can be usefully and legally folded into it as a post-indexing operation.
   ///
   /// \returns true if a candidate is found.
   bool findPostIndexCandidate(MachineInstr &MI, Register &Addr, Register &Base,
                               Register &Offset);
 

llvm/include/llvm/Target/GlobalISel/Combine.td

 // Fold x op 1 -> x
 def right_identity_one: GICombineRule<
   (defs root:$root),
   (match (wip_match_opcode G_MUL):$root,
     [{ return Helper.matchConstantOp(${root}->getOperand(2), 1); }]),
   (apply [{ return Helper.replaceSingleDefInstWithOperand(*${root}, 1); }])
 >;
 
+// Fold x op 1.0 -> x
+def right_identity_fp_one: GICombineRule<
+  (defs root:$root),
+  (match (wip_match_opcode G_FMUL):$root,
+    [{ return Helper.matchFConstantOp(${root}->getOperand(2), 1.0); }]),
+  (apply [{ return Helper.replaceSingleDefInstWithOperand(*${root}, 1); }])
+>;
+
 // Fold (x op x) - > x
 def binop_same_val: GICombineRule<
   (defs root:$root),
   (match (wip_match_opcode G_AND, G_OR):$root,
     [{ return Helper.matchBinOpSameVal(*${root}); }]),
   (apply [{ return Helper.replaceSingleDefInstWithOperand(*${root}, 1); }])
 >;
 

 
 def combine_minmax_nan_matchinfo: GIDefMatchData<"unsigned">;
 def combine_minmax_nan: GICombineRule<
   (defs root:$root, combine_minmax_nan_matchinfo:$info),
   (match (wip_match_opcode G_FMINNUM, G_FMAXNUM, G_FMINIMUM, G_FMAXIMUM):$root,
          [{ return Helper.matchCombineFMinMaxNaN(*${root}, ${info}); }]),
   (apply [{ Helper.replaceSingleDefInstWithOperand(*${root}, ${info}); }])>;
 
+// Transform (fmul (fneg x), (fneg y)) -> (fmul x, y)
+def fmul_fneg_fneg_matchinfo : GIDefMatchData<"std::pair<Register, Register>">;
+def fmul_fneg_fneg: GICombineRule <
+  (defs root:$root, fmul_fneg_fneg_matchinfo:$matchinfo),
+  (match (wip_match_opcode G_FMUL):$root,
+         [{ return Helper.matchFMulWithNegatedInputs(*${root},
+                                                     ${matchinfo}); }]),
+  (apply [{ return Helper.applyFMulWithNegatedInputs(*${root},
+                                                     ${matchinfo}); }])>;
+
 // FIXME: These should use the custom predicate feature once it lands.
 def undef_combines : GICombineGroup<[undef_to_fp_zero, undef_to_int_zero,
                                      undef_to_negative_one,
                                      binop_left_undef_to_zero,
                                      binop_right_undef_to_undef,
                                      propagate_undef_any_op,
                                      propagate_undef_all_ops,
                                      propagate_undef_shuffle_mask,
                                      erase_undef_store,
                                      unmerge_undef]>;
 
 def identity_combines : GICombineGroup<[select_same_val, right_identity_zero,
                                         binop_same_val, binop_left_to_zero,
                                         binop_right_to_zero, p2i_to_i2p,
                                         i2p_to_p2i, anyext_trunc_fold,
-                                        fneg_fneg_fold, right_identity_one]>;
+                                        fneg_fneg_fold, right_identity_one,
+                                        right_identity_fp_one]>;
 
 def const_combines : GICombineGroup<[constant_fp_op, const_ptradd_to_i2p,
                                      overlapping_and, mulo_by_2, mulo_by_0,
                                      addo_by_0, combine_minmax_nan]>;
 
 def known_bits_simplifications : GICombineGroup<[
   redundant_and, redundant_sext_inreg, redundant_or, urem_pow2_to_mask,
   zext_trunc_fold, icmp_to_true_false_known_bits, icmp_to_lhs_known_bits]>;
def trivial_combines : GICombineGroup<[copy_prop, mul_to_shl, add_p2i_to_ptradd,
                                        mul_by_neg_one]>;
 
 def fma_combines : GICombineGroup<[combine_fadd_fmul_to_fmad_or_fma,
   combine_fadd_fpext_fmul_to_fmad_or_fma, combine_fadd_fma_fmul_to_fmad_or_fma,
   combine_fadd_fpext_fma_fmul_to_fmad_or_fma, combine_fsub_fmul_to_fmad_or_fma,
   combine_fsub_fneg_fmul_to_fmad_or_fma, combine_fsub_fpext_fmul_to_fmad_or_fma,
   combine_fsub_fpext_fneg_fmul_to_fmad_or_fma]>;
 
+def fp_combines : GICombineGroup<[fma_combines, fmul_fneg_fneg]>;
+
 def all_combines : GICombineGroup<[trivial_combines, insert_vec_elt_combines,
     extract_vec_elt_combines, combines_for_extload,
     combine_indexed_load_store, undef_combines, identity_combines, phi_combines,
     simplify_add_to_sub, hoist_logic_op_with_same_opcode_hands,
     reassocs, ptr_add_immed_chain,
     shl_ashr_to_sext_inreg, sext_inreg_of_load,
     width_reduction_combines, select_combines,
     known_bits_simplifications, ext_ext_fold,
     not_cmp_fold, opt_brcond_by_inverting_cond,
     unmerge_merge, fabs_fabs_fold, unmerge_cst, unmerge_dead_to_trunc,
     unmerge_zext_to_zext, merge_unmerge, trunc_ext_fold, trunc_shl,
     const_combines, xor_of_and_with_same_reg, ptr_add_with_zero,
     shift_immed_chain, shift_of_shifted_logic_chain, load_or_combine,
     truncstore_merge, div_rem_to_divrem, funnel_shift_combines,
     form_bitfield_extract, constant_fold, fabs_fneg_fold,
     intdiv_combines, mulh_combines, redundant_neg_operands,
-    and_or_disjoint_mask, fma_combines, fold_binop_into_select]>;
+    and_or_disjoint_mask, fp_combines, fold_binop_into_select]>;
 
 // A combine group used to for prelegalizer combiners at -O0. The combines in
 // this group have been selected based on experiments to balance code size and
 // compile time performance.
 def optnone_combines : GICombineGroup<[trivial_combines,
     ptr_add_immed_chain, combines_for_extload,
     not_cmp_fold, opt_brcond_by_inverting_cond]>;

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

bool CombinerHelper::matchConstantOp(const MachineOperand &MOP, int64_t C) {
   if (!MOP.isReg())
     return false;
   auto *MI = MRI.getVRegDef(MOP.getReg());
   auto MaybeCst = isConstantOrConstantSplatVector(*MI, MRI);
   return MaybeCst.hasValue() && MaybeCst->getBitWidth() <= 64 &&
          MaybeCst->getSExtValue() == C;
 }
 
+bool CombinerHelper::matchFConstantOp(const MachineOperand &MOP, double C) {
+  if (!MOP.isReg())
+    return false;
+  const ConstantFP *FCst = getConstantFPVRegVal(MOP.getReg(), MRI);
+  return FCst && FCst->getValueAPF().isExactlyValue(C);
+}
+
 bool CombinerHelper::replaceSingleDefInstWithOperand(MachineInstr &MI,
                                                      unsigned OpIdx) {
   assert(MI.getNumExplicitDefs() == 1 && "Expected one explicit def?");
   Register OldReg = MI.getOperand(0).getReg();
   Register Replacement = MI.getOperand(OpIdx).getReg();
   assert(canReplaceReg(OldReg, Replacement, MRI) && "Cannot replace register?");
   MI.eraseFromParent();
   replaceRegWith(MRI, OldReg, Replacement);
if (!MaybeCst || !MaybeCst->getValueAPF().isNaN())
       return false;
     IdxToPropagate = PropagateNaN ? Idx : (Idx == 1 ? 2 : 1);
     return true;
   };
 
   return MatchNaN(1) || MatchNaN(2);
 }
 
+bool CombinerHelper::matchFMulWithNegatedInputs(
+    MachineInstr &MI, std::pair<Register, Register> &MatchInfo) {
+  assert(MI.getOpcode() == TargetOpcode::G_FMUL && "Expected a G_FMUL");
+  Register Dst = MI.getOperand(0).getReg();
+  Register Src0;
+  Register Src1;
+
+  if (mi_match(Dst, MRI, m_GFMul(m_GFNeg(m_Reg(Src0)), m_GFNeg(m_Reg(Src1))))) {
+    MatchInfo = std::make_pair(Src0, Src1);
+    return true;
+  }
+  return false;
+}
+
+bool CombinerHelper::applyFMulWithNegatedInputs(
+    MachineInstr &MI, std::pair<Register, Register> &MatchInfo) {
+  assert(MI.getOpcode() == TargetOpcode::G_FMUL && "Expected a G_FMUL");
+  Register Dst = MI.getOperand(0).getReg();
+  Register Src0 = MatchInfo.first;
+  Register Src1 = MatchInfo.second;
+
+  Builder.setInstrAndDebugLoc(MI);
+  Builder.buildFMul({Dst}, Src0, Src1, MI.getFlags());
+  MI.eraseFromParent();
+  return true;
+}
+
 bool CombinerHelper::tryCombine(MachineInstr &MI) {
   if (tryCombineCopy(MI))
     return true;
   if (tryCombineExtendingLoads(MI))
     return true;
   if (tryCombineIndexedLoadStore(MI))
     return true;
   return false;
 }

llvm/test/CodeGen/AArch64/GlobalISel/combine-fmul.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -run-pass=aarch64-prelegalizer-combiner -verify-machineinstrs -mtriple aarch64-unknown-unknown %s -o - | FileCheck %s
+
+---
+name:            fmul_by_one
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0
+    ; CHECK-LABEL: name: fmul_by_one
+    ; CHECK: liveins: $x0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
+    ; CHECK-NEXT: $x0 = COPY [[COPY]](s64)
+    %0:_(s64) = COPY $x0
+    %1:_(s64) = G_FCONSTANT double 1.0
+    %2:_(s64) = G_FMUL %0, %1(s64)
+    $x0 = COPY %2(s64)
+...
+---
+name:            mul_vector_by_one
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $q0
+    ; Currently not implemented.
+    ; CHECK-LABEL: name: mul_vector_by_one
+    ; CHECK: liveins: $q0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_FCONSTANT float 1.000000e+00
+    ; CHECK-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
+    ; CHECK-NEXT: [[FMUL:%[0-9]+]]:_(<4 x s32>) = G_FMUL [[COPY]], [[BUILD_VECTOR]]
+    ; CHECK-NEXT: $q0 = COPY [[FMUL]](<4 x s32>)
+    %0:_(<4 x s32>) = COPY $q0
+    %1:_(s32) = G_FCONSTANT float 1.0
+    %2:_(<4 x s32>) = G_BUILD_VECTOR %1(s32), %1(s32), %1(s32), %1(s32)
+    %3:_(<4 x s32>) = G_FMUL %0, %2(<4 x s32>)
+    $q0 = COPY %3(<4 x s32>)
+...
+---
+name:            fmul_fneg_fneg
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0
+    ; CHECK-LABEL: name: fmul_fneg_fneg
+    ; CHECK: liveins: $x0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
+    ; CHECK-NEXT: [[FMUL:%[0-9]+]]:_(s64) = G_FMUL [[COPY]], [[COPY]]
+    ; CHECK-NEXT: $x0 = COPY [[FMUL]](s64)
+    %0:_(s64) = COPY $x0
+    %1:_(s64) = G_FNEG %0(s64)
+    %2:_(s64) = G_FMUL %1(s64), %1(s64)
+    $x0 = COPY %2(s64)
+...
+---
+name:            fmul_vector_fneg_fneg
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $q0
+    ; Currently not implemented.
+    ; CHECK-LABEL: name: fmul_vector_fneg_fneg
+    ; CHECK: liveins: $q0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
+    ; CHECK-NEXT: [[FMUL:%[0-9]+]]:_(<4 x s32>) = G_FMUL [[COPY]], [[COPY]]
+    ; CHECK-NEXT: $q0 = COPY [[FMUL]](<4 x s32>)
+    %0:_(<4 x s32>) = COPY $q0
+    %1:_(<4 x s32>) = G_FNEG %0(<4 x s32>)
+    %2:_(<4 x s32>) = G_FMUL %1(<4 x s32>), %1(<4 x s32>)
+    $q0 = COPY %2(<4 x s32>)
+...