[GISel] Add new GISel combiners for G_MUL

[mkitzan]

Summary

Patch adds two new GICombinerRules, one for G_MUL(X, 1) and another for G_MUL(X, -1). G_MUL(X, 1) is an identity combine, and G_MUL(X, -1) gets replaced with G_SUB(0, X). Patch additionally adds new combiner tests for the AArch64 target to test these new combiner rules, as well as updates AMDGPU GISel tests.

[mkitzan]

• Rebased and fixed merge conflicts

[paquette]

In the future, can you do a full-context diff? It lets reviewers see the full file.

e.g.

git show -U999999

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

441

This could probably be a right_identity_one combine, similar to right_identity_zero. Then you can add any other operators where this is true to the wip_match_opcode list in a follow-up.

453

Could this use matchConstantOp?

[mkitzan]

• Changed mul_by_one to right_identity_one
• Changed mul_by_neg_one to use matchConstantOp

[paquette]

LGTM

[mkitzan]

Thanks for the review. Still don't have commit access, will need someone to commit on my behalf. Thanks in advance!

[aditya_nandakumar]

In D87668#2275249, @mkitzan wrote:

Thanks for the review. Still don't have commit access, will need someone to commit on my behalf. Thanks in advance!

I can commit for you this time as well. Can you email Chris to request for access as mentioned here? - https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

[mkitzan]

Thanks again Aditya! I'll reach out to Chris about commit access.

[aditya_nandakumar]

Pushed in 97203cfd6ba

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

public:
 
   /// Transform fneg(fneg(x)) to x.
   bool matchCombineFNegOfFNeg(MachineInstr &MI, Register &Reg);
 
   /// Match fabs(fabs(x)) to fabs(x).
   bool matchCombineFAbsOfFAbs(MachineInstr &MI, Register &Src);
   bool applyCombineFAbsOfFAbs(MachineInstr &MI, Register &Src);
 
+  /// Transform G_MUL(x, -1) to G_SUB(0, x)
+  bool applyCombineMulByNegativeOne(MachineInstr &MI);
+
   /// Return true if any explicit use operand on \p MI is defined by a
   /// G_IMPLICIT_DEF.
   bool matchAnyExplicitUseIsUndef(MachineInstr &MI);
 
   /// Return true if all register explicit use operands on \p MI are defined by
   /// a G_IMPLICIT_DEF.
   bool matchAllExplicitUsesAreUndef(MachineInstr &MI);
 

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

 // Fold x op 0 -> x
 def right_identity_zero: GICombineRule<
   (defs root:$root),
   (match (wip_match_opcode G_SUB, G_ADD, G_OR, G_XOR, G_SHL, G_ASHR, G_LSHR):$root,
     [{ return Helper.matchConstantOp(${root}->getOperand(2), 0); }]),
   (apply [{ return Helper.replaceSingleDefInstWithOperand(*${root}, 1); }])
 >;
 
+// 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 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 unmerge_dead_to_trunc : GICombineRule<
   (defs root:$d),
   (match (wip_match_opcode G_UNMERGE_VALUES): $d,
   [{ return Helper.matchCombineUnmergeWithDeadLanesToTrunc(*${d}); }]),
   (apply [{ return Helper.applyCombineUnmergeWithDeadLanesToTrunc(*${d}); }])
 >;
 
 // Transform x,y = unmerge(zext(z)) -> x = zext z; y = 0.
 def unmerge_zext_to_zext : GICombineRule<
   (defs root:$d),
   (match (wip_match_opcode G_UNMERGE_VALUES): $d,
   [{ return Helper.matchCombineUnmergeZExtToZExt(*${d}); }]),
   (apply [{ return Helper.applyCombineUnmergeZExtToZExt(*${d}); }])
 >;
 
+// Transform (mul x, -1) -> (sub 0, x)
+def mul_by_neg_one: GICombineRule <
+  (defs root:$root),
+  (match (wip_match_opcode G_MUL):$root,
+         [{ return Helper.matchConstantOp(${root}->getOperand(2), -1); }]),
+  (apply [{ return Helper.applyCombineMulByNegativeOne(*${root}); }])
+>;
+
 // 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,
                                      propagate_undef_any_op,
                                      propagate_undef_all_ops,
                                      propagate_undef_shuffle_mask,
                                      erase_undef_store]>;
 
 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]>;
+                                        fneg_fneg_fold, right_identity_one]>;
 
 def known_bits_simplifications : GICombineGroup<[
   and_trivial_mask, redundant_sext_inreg]>;
 
 def width_reduction_combines : GICombineGroup<[reduce_shl_of_extend]>;
 
 def select_combines : GICombineGroup<[select_undef_cmp, select_constant_cmp]>;
 
-def trivial_combines : GICombineGroup<[copy_prop, mul_to_shl, add_p2i_to_ptradd]>;
+def trivial_combines : GICombineGroup<[copy_prop, mul_to_shl, add_p2i_to_ptradd,
+                                       mul_by_neg_one]>;
+
 def all_combines : GICombineGroup<[trivial_combines, ptr_add_immed_chain,
     combines_for_extload, combine_indexed_load_store, undef_combines,
     identity_combines, simplify_add_to_sub,
     hoist_logic_op_with_same_opcode_hands,
     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]>;

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

if (MI.getOpcode() == TargetOpcode::G_ANYEXT ||
     Builder.buildInstr(SrcExtOp, {DstReg}, {Reg});
     MI.eraseFromParent();
     return true;
   }
 
   return false;
 }
 
+bool CombinerHelper::applyCombineMulByNegativeOne(MachineInstr &MI) {
+  assert(MI.getOpcode() == TargetOpcode::G_MUL && "Expected a G_MUL");
+  Register DstReg = MI.getOperand(0).getReg();
+  Register SrcReg = MI.getOperand(1).getReg();
+  LLT DstTy = MRI.getType(DstReg);
+
+  Builder.setInstrAndDebugLoc(MI);
+  Builder.buildSub(DstReg, Builder.buildConstant(DstTy, 0), SrcReg,
+                   MI.getFlags());
+  MI.eraseFromParent();
+  return true;
+}
+
 bool CombinerHelper::matchCombineFNegOfFNeg(MachineInstr &MI, Register &Reg) {
   assert(MI.getOpcode() == TargetOpcode::G_FNEG && "Expected a G_FNEG");
   Register SrcReg = MI.getOperand(1).getReg();
   return mi_match(SrcReg, MRI, m_GFNeg(m_Reg(Reg)));
 }
 
 bool CombinerHelper::matchCombineFAbsOfFAbs(MachineInstr &MI, Register &Src) {
   assert(MI.getOpcode() == TargetOpcode::G_FABS && "Expected a G_FABS");

llvm/test/CodeGen/AArch64/GlobalISel/combine-mul.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:            mul_by_zero
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0
+    ; CHECK-LABEL: name: mul_by_zero
+    ; CHECK: liveins: $x0
+    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
+    ; CHECK: $x0 = COPY [[C]](s64)
+    %0:_(s64) = COPY $x0
+    %1:_(s64) = G_CONSTANT i64 0
+    %2:_(s64) = G_MUL %0, %1(s64)
+    $x0 = COPY %2(s64)
+...
+---
+name:            mul_vector_by_zero
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $q0
+    ; Currently not implemented.
+    ; CHECK-LABEL: name: mul_vector_by_zero
+    ; CHECK: liveins: $q0
+    ; CHECK: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+    ; CHECK: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
+    ; CHECK: [[MUL:%[0-9]+]]:_(<4 x s32>) = G_MUL [[COPY]], [[BUILD_VECTOR]]
+    ; CHECK: $q0 = COPY [[MUL]](<4 x s32>)
+    %0:_(<4 x s32>) = COPY $q0
+    %1:_(s32) = G_CONSTANT i32 0
+    %2:_(<4 x s32>) = G_BUILD_VECTOR %1(s32), %1(s32), %1(s32), %1(s32)
+    %3:_(<4 x s32>) = G_MUL %0, %2(<4 x s32>)
+    $q0 = COPY %3(<4 x s32>)
+...
+---
+name:            mul_by_one
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0
+    ; CHECK-LABEL: name: mul_by_one
+    ; CHECK: liveins: $x0
+    ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
+    ; CHECK: $x0 = COPY [[COPY]](s64)
+    %0:_(s64) = COPY $x0
+    %1:_(s64) = G_CONSTANT i64 1
+    %2:_(s64) = G_MUL %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: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
+    ; CHECK: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
+    ; CHECK: [[MUL:%[0-9]+]]:_(<4 x s32>) = G_MUL [[COPY]], [[BUILD_VECTOR]]
+    ; CHECK: $q0 = COPY [[MUL]](<4 x s32>)
+    %0:_(<4 x s32>) = COPY $q0
+    %1:_(s32) = G_CONSTANT i32 1
+    %2:_(<4 x s32>) = G_BUILD_VECTOR %1(s32), %1(s32), %1(s32), %1(s32)
+    %3:_(<4 x s32>) = G_MUL %0, %2(<4 x s32>)
+    $q0 = COPY %3(<4 x s32>)
+...
+---
+name:            mul_by_neg_one
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0
+    ; CHECK-LABEL: name: mul_by_neg_one
+    ; CHECK: liveins: $x0
+    ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
+    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
+    ; CHECK: [[SUB:%[0-9]+]]:_(s64) = G_SUB [[C]], [[COPY]]
+    ; CHECK: $x0 = COPY [[SUB]](s64)
+    %0:_(s64) = COPY $x0
+    %1:_(s64) = G_CONSTANT i64 -1
+    %2:_(s64) = G_MUL %0, %1(s64)
+    $x0 = COPY %2(s64)
+...
+---
+name:            mul_vector_by_neg_one
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $q0
+    ; Currently not implemented.
+    ; CHECK-LABEL: name: mul_vector_by_neg_one
+    ; CHECK: liveins: $q0
+    ; CHECK: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 -1
+    ; CHECK: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
+    ; CHECK: [[MUL:%[0-9]+]]:_(<4 x s32>) = G_MUL [[COPY]], [[BUILD_VECTOR]]
+    ; CHECK: $q0 = COPY [[MUL]](<4 x s32>)
+    %0:_(<4 x s32>) = COPY $q0
+    %1:_(s32) = G_CONSTANT i32 -1
+    %2:_(<4 x s32>) = G_BUILD_VECTOR %1(s32), %1(s32), %1(s32), %1(s32)
+    %3:_(<4 x s32>) = G_MUL %0, %2(<4 x s32>)
+    $q0 = COPY %3(<4 x s32>)
+...