[llvm][CodeGen] Update checking method of loop-carried phi in window scheduler #96288
Conversation
…scheduler Added some logic to check loop-carried phis in the window scheduler. It now includes the scenario where the preceding phi uses the virtual register defined by the succeeding phi.
|
During our internal code review, we discovered that we had missed an equivalent loop-carried phi scenario. Previously, we checked the scenario where: "The virtual register defined by the preceding phi is used by the succeeding phi." There is an equivalent scenario where: "The preceding phi uses the virtual register defined by the succeeding phi." This patch improves the check to cover this scenario. |
|
Could you please review this patch? Thank you~ @arsenm |
llvm/lib/CodeGen/WindowScheduler.cpp
Outdated
| if (!MO.isReg()) | ||
| continue; | ||
| if (MO.isDef()) { |
There was a problem hiding this comment.
You know the structure of phi so you can increment by 2 and check the single def separately
| SmallSet<Register, 8> PrevDefs; | ||
| SmallSet<Register, 8> PrevUses; |
There was a problem hiding this comment.
These sets aren't used outside of the lambda. Just make the lambda a helper function with these used locally?
There was a problem hiding this comment.
These sets are used to record the defs and uses of phis that have already been checked in the current MBB.
They need to be initialized and retain register information across multiple calls to the lambda.
Therefore, wouldn't it be better to place these sets outside the lambda?
| SmallSet<Register, 8> PrevDefs; | ||
| SmallSet<Register, 8> PrevUses; |
llvm/lib/CodeGen/WindowScheduler.cpp
Outdated
| // preceding phi is used by the succeeding phi;(2)The preceding phi uses the | ||
| // virtual register defined by the succeeding phi. | ||
| auto Def = Phi.getOperand(0); | ||
| if (Def.isReg()) { |
There was a problem hiding this comment.
Def must be a register. Don't try to handle invalid MIR
llvm/lib/CodeGen/WindowScheduler.cpp
Outdated
| } | ||
| for (unsigned I = 1, E = Phi.getNumOperands(); I != E; I += 2) { | ||
| auto Use = Phi.getOperand(I); | ||
| if (Use.isReg()) { |
…scheduler (llvm#96288) Added some logic to check loop-carried phis in the window scheduler. It now includes the scenario where the preceding phi uses the virtual register defined by the succeeding phi.
Added some logic to check loop-carried phis in the window scheduler. It
now includes the scenario where the preceding phi uses the virtual
register defined by the succeeding phi.