[llvm] [MachineCombiner] Don't ignore PHI depths (PR #82025)

Jonas Paulsson via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 12 07:56:43 PDT 2024


JonPsson1 wrote:

> > This means that Root currenty has that depth included in it, but NewRoot ignores it
> 
> That sounds completely wrong.
> 
> > This patch simply removes that check for PHIs so that they are also counted.
> 
> What about loops? Have you had a chance to check how PHIs will be calculated for them?

That's a good question.. :)   I did an experiment with this and found that the PHI with a long-latency incoming edge (fdiv) had a big depth value if outside of any loop, but 0 depth if I put it in a loop. So it seems that inside a loop this works as expected with basically 0 depth for any phi in the header, while outside of a loop the phi should have the incoming depth (for both Root and NewRoot with this patch). 

I think that is what this code does:

```
MinInstrCountEnsemble::pickTracePred(const MachineBasicBlock *MBB) {
  if (MBB->pred_empty())
    return nullptr;
  const MachineLoop *CurLoop = getLoopFor(MBB);
  // Don't leave loops, and never follow back-edges.
  if (CurLoop && MBB == CurLoop->getHeader())
    return nullptr;
...

and

MinInstrCountEnsemble::pickTraceSucc(const MachineBasicBlock *MBB) {
  if (MBB->succ_empty())
    return nullptr;
  const MachineLoop *CurLoop = getLoopFor(MBB);
  const MachineBasicBlock *Best = nullptr;
  unsigned BestHeight = 0;
  for (const MachineBasicBlock *Succ : MBB->successors()) {
    // Don't consider back-edges.
    if (CurLoop && Succ == CurLoop->getHeader())
      continue;
...
```
IIUC, this makes sure that any PHI in the loop header specifically does not carry a trace from the predecessor - preheader or latch included, which is what I observed per above.



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


More information about the llvm-commits mailing list