Address reviewer comments

Author: dc03-work

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

@@ -222,7 +222,7 @@ def idempotent_prop : GICombineRule<
 
 // Convert freeze(Op(Op0, NonPoisonOps...)) to Op(freeze(Op0), NonPoisonOps...)
 // when Op0 is not guaranteed non-poison
-def push_freeze_to_prevent_poison_propagation : GICombineRule<
+def push_freeze_to_prevent_poison_from_propagating : GICombineRule<
   (defs root:$root, build_fn_matchinfo:$matchinfo),
   (match (G_FREEZE $dst, $src):$root,
          [{ return !isGuaranteedNotToBePoison(${src}.getReg(), MRI) && Helper.matchFreezeOfSingleMaybePoisonOperand(*${root}, ${matchinfo}); }]),
@@ -1721,7 +1721,7 @@ def all_combines : GICombineGroup<[trivial_combines, vector_ops_combines,
     fsub_to_fneg, commute_constant_to_rhs, match_ands, match_ors,
     combine_concat_vector, double_icmp_zero_and_or_combine, match_addos,
     sext_trunc, zext_trunc, combine_shuffle_concat,
-    push_freeze_to_prevent_poison_propagation]>;
+    push_freeze_to_prevent_poison_from_propagating]>;
 
 // 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

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

@@ -225,15 +225,22 @@ void CombinerHelper::applyCombineCopy(MachineInstr &MI) {
 
 bool CombinerHelper::matchFreezeOfSingleMaybePoisonOperand(
     MachineInstr &MI, BuildFnTy &MatchInfo) {
-  // Ported from InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating
+  // Ported from InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating.
   Register DstOp = MI.getOperand(0).getReg();
   Register OrigOp = MI.getOperand(1).getReg();
 
   if (!MRI.hasOneNonDBGUse(OrigOp))
     return false;
 
   MachineInstr *OrigDef = MRI.getUniqueVRegDef(OrigOp);
-  // Avoid trying to fold G_PHI and G_UNMERGE_VALUES.
+  // Even if only a single operand of the PHI is not guaranteed non-poison,
+  // moving freeze() backwards across a PHI can cause optimization issues for
+  // other users of that operand.
+  //
+  // Moving freeze() from one of the output registers of a G_UNMERGE_VALUES to
+  // the source register is unprofitable because it makes the freeze() more
+  // strict than is necessary (it would affect the whole register instead of
+  // just the subreg being frozen).
   if (OrigDef->isPHI() || isa<GUnmerge>(OrigDef))
     return false;
 
@@ -243,7 +250,6 @@ bool CombinerHelper::matchFreezeOfSingleMaybePoisonOperand(
 
   std::optional<MachineOperand> MaybePoisonOperand;
   for (MachineOperand &Operand : OrigDef->uses()) {
-    // Avoid working on non-register operands.
     if (!Operand.isReg())
       return false;