[PATCH] D126523: [MachineSSAUpdater] Re-use existing phi node in GetValueInMiddleOfBlock

[Serguei Katkov via Phabricator via llvm-commits]

skatkov created this revision.
skatkov added reviewers: StephenTozer, reames, dantrushin, asbirlea, djtodoro.
Herald added a subscriber: hiraditya.
Herald added a project: All.
skatkov requested review of this revision.
Herald added a project: LLVM.

The motivation is actually in MachineSSAUpdater but I've updated SSSUpdater as well.

GetValueInMiddleOfBlock uses result of GetValueAtEndOfBlockInternal if there is no register
defined for current basic block.

If there is already a register it tries (in this order):

- to find single register coming from all predecessors
- find existing phi node which matches our incoming registers
- build new phi.

In the attached example it results in STATEPOPINT instruction had two uses of some register
and after tail duplication transformation these two uses are rewritten by two different registers.

It happened because for the first use SSAUpdater used the PHI node returned from GetValueAtEndOfBlockInternal.
For the second use the block already has assigned register and it is able to find singular value which is different
from build PHI node.

If singular value was not found we would probably find the matching phi node built for the first use.
But it is redundant compile time. Instead the proposal is

- If there is a register for this BB and it is defined out of current BB or it is a PHI register then we could re-use this register as it is defined in the middle basic block.


https://reviews.llvm.org/D126523

Files:
  llvm/lib/CodeGen/MachineSSAUpdater.cpp
  llvm/lib/Transforms/Utils/SSAUpdater.cpp
  llvm/test/CodeGen/AArch64/tail-dup-redundant-phi.mir


Index: llvm/test/CodeGen/AArch64/tail-dup-redundant-phi.mir
===================================================================
--- llvm/test/CodeGen/AArch64/tail-dup-redundant-phi.mir
+++ llvm/test/CodeGen/AArch64/tail-dup-redundant-phi.mir
@@ -343,7 +343,7 @@
   ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
   ; CHECK-NEXT:   [[MOVi32imm5:%[0-9]+]]:gpr32 = MOVi32imm 10
   ; CHECK-NEXT:   $w0 = COPY [[MOVi32imm5]]
-  ; CHECK-NEXT:   [[STATEPOINT2:%[0-9]+]]:gpr64all, [[STATEPOINT3:%[0-9]+]]:gpr64all = STATEPOINT 2882400000, 0, 1, @wombat, $w0, 2, 0, 2, 0, 2, 3, [[PHI3]], [[PHI2]], [[PHI]], 2, 2, [[PHI1]](tied-def 0), [[PHI]](tied-def 1), 2, 0, 2, 2, 0, 0, 1, 1, csr_aarch64_aapcs, implicit-def $sp, implicit-def dead early-clobber $lr
+  ; CHECK-NEXT:   [[STATEPOINT2:%[0-9]+]]:gpr64all, [[STATEPOINT3:%[0-9]+]]:gpr64all = STATEPOINT 2882400000, 0, 1, @wombat, $w0, 2, 0, 2, 0, 2, 3, [[PHI3]], [[PHI2]], [[PHI2]], 2, 2, [[PHI3]](tied-def 0), [[PHI2]](tied-def 1), 2, 0, 2, 2, 0, 0, 1, 1, csr_aarch64_aapcs, implicit-def $sp, implicit-def dead early-clobber $lr
   ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.18.bb51:
Index: llvm/lib/Transforms/Utils/SSAUpdater.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SSAUpdater.cpp
+++ llvm/lib/Transforms/Utils/SSAUpdater.cpp
@@ -100,6 +100,14 @@
   if (!HasValueForBlock(BB))
     return GetValueAtEndOfBlock(BB);
 
+  // Ok, we have already got a value for this block. If it is out of our block
+  // or it is a phi - we can re-use it as it will be defined in the middle block
+  // as well.
+  Value *defV = FindValueForBlock(BB);
+  if (auto I = dyn_cast<Instruction>(defV))
+    if (isa<PHINode>(I) || I->getParent() != BB)
+      return defV;
+
   // Otherwise, we have the hard case.  Get the live-in values for each
   // predecessor.
   SmallVector<std::pair<BasicBlock *, Value *>, 8> PredValues;
Index: llvm/lib/CodeGen/MachineSSAUpdater.cpp
===================================================================
--- llvm/lib/CodeGen/MachineSSAUpdater.cpp
+++ llvm/lib/CodeGen/MachineSSAUpdater.cpp
@@ -152,6 +152,14 @@
   if (!HasValueForBlock(BB))
     return GetValueAtEndOfBlockInternal(BB, ExistingValueOnly);
 
+  // Ok, we have already got a value for this block. If it is out of our block
+  // or it is a phi - we can re-use it as it will be defined in the middle block
+  // as well.
+  Register defR = getAvailableVals(AV).lookup(BB);
+  if (auto I = MRI->getVRegDef(defR))
+    if (I->isPHI() || I->getParent() != BB)
+      return defR;
+
   // If there are no predecessors, just return undef.
   if (BB->pred_empty()) {
     // If we cannot insert new instructions, just return $noreg.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126523.432475.patch
Type: text/x-patch
Size: 2816 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220527/69be6450/attachment.bin>