[llvm] [GISel][CombinerHelper] Push freeze through non-poison-producing operands (PR #90618)

Dhruv Chawla via llvm-commits llvm-commits at lists.llvm.org
Wed May 22 22:55:26 PDT 2024


================
@@ -223,6 +223,64 @@ void CombinerHelper::applyCombineCopy(MachineInstr &MI) {
   replaceRegWith(MRI, DstReg, SrcReg);
 }
 
+bool CombinerHelper::matchFreezeOfSingleMaybePoisonOperand(
+    MachineInstr &MI, BuildFnTy &MatchInfo) {
+  // Ported from InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating
+  Register DstOp = MI.getOperand(0).getReg();
+  Register OrigOp = MI.getOperand(1).getReg();
+
+  if (OrigOp.isPhysical() || !MRI.hasOneNonDBGUse(OrigOp))
+    return false;
+
+  MachineInstr *OrigDef = MRI.getUniqueVRegDef(OrigOp);
+  // Avoid trying to fold G_PHI, G_UNMERGE_VALUES, G_FREEZE (the latter is
+  // handled by idempotent_prop)
+  if (!OrigDef || OrigDef->isPHI() || isa<GUnmerge>(OrigDef) ||
+      isa<GFreeze>(OrigDef))
+    return false;
+
+  if (canCreateUndefOrPoison(OrigOp, MRI))
+    return false;
+
+  std::optional<MachineOperand> MaybePoisonOperand = std::nullopt;
+  for (MachineOperand &Operand : OrigDef->uses()) {
+    // Avoid working on non-register operands or physical registers.
+    if (!Operand.isReg() || Operand.getReg().isPhysical())
+      return false;
+
+    if (isGuaranteedNotToBeUndefOrPoison(Operand.getReg(), MRI))
+      continue;
+
+    if (!MaybePoisonOperand)
+      MaybePoisonOperand = Operand;
+    // We have more than one maybe-poison operand. Moving the freeze is unsafe.
+    else
+      return false;
+  }
+
+  // Eliminate freeze if all operands are guaranteed non-poison
+  if (!MaybePoisonOperand) {
+    MatchInfo = [=](MachineIRBuilder &B) { MRI.replaceRegWith(DstOp, OrigOp); };
+    return true;
+  }
+
+  if (!MaybePoisonOperand->isReg())
+    return false;
+
+  Register MaybePoisonOperandReg = MaybePoisonOperand->getReg();
+  LLT MaybePoisonOperandRegTy = MRI.getType(MaybePoisonOperandReg);
+
+  MatchInfo = [=](MachineIRBuilder &B) mutable {
+    auto Reg = MRI.createGenericVirtualRegister(MaybePoisonOperandRegTy);
+    B.setInsertPt(*OrigDef->getParent(), OrigDef->getIterator());
----------------
dc03-work wrote:

No, `OrigDef` is the instruction that is being frozen. This function gets called on `freeze(op(op0, ops...))`, where `OrigDef` is `op` and the insert point is before the `freeze` but after `op`. We have to transform this to `op(freeze(op0), ops...)`, so it necessarily requires moving the insert point before `op`.

Removing the `setInsertPt` call causes the following tests to fail:
```
Failed Tests (2):
  LLVM :: CodeGen/AArch64/GlobalISel/combine-select.mir
  LLVM :: CodeGen/AMDGPU/div_v2i128.ll
```
Where the first test has `freeze`s inserted in the wrong place, for example:
```
        683:   
        684:  %0:_(s64) = COPY $x0 
        685:  %2:_(s64) = COPY $x2 
        686:  %c:_(s1) = G_TRUNC %0(s64) 
        687:  %f:_(s1) = G_TRUNC %11(s64) 
        688:  %11:_(s64) = G_FREEZE %2 
next:147      !~~~~~~~~~~~~~~~~~~~~~~~  error: match on wrong line
        689:  %sel:_(s1) = G_OR %c, %f 
        690:  %ext:_(s32) = G_ANYEXT %sel(s1) 
        691:  $w0 = COPY %ext(s32) 
        692:  
        693: ... 
```
and the second test fails in the machine verifier:
```
*** Bad machine code: Virtual register defs don't dominate all uses. ***
- function:    v_sdiv_v2i128_vv
- v. register: %964
```

https://github.com/llvm/llvm-project/pull/90618


More information about the llvm-commits mailing list