[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