[RFC][AIE2] Add an IR Phi reordering pass #128
Conversation
This pass helps Greedy to avoid spills.
andcarminati
requested review from
abhinay-anubola,
abnikant,
gbossu,
khallouh,
konstantinschwarz,
martien-de-jong,
SagarMaheshwari99 and
stephenneuendorffer
as code owners
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "AIE2.h" |
There was a problem hiding this comment.
If it is AIE2 specific, consider renaming to AIE2PHISorterPass. Otherwise, try to avoid including AIE2.h
| } | ||
|
|
||
| if (WorkList.empty()) | ||
| continue; |
There was a problem hiding this comment.
I guess InstrOrder could be declared after this continue.
| for (BasicBlock *BB : PHI.blocks()) { | ||
| // Check basically if this loop block | ||
| // is part of the body. | ||
| if (BB == &B) |
There was a problem hiding this comment.
I'm wondering why you use LoopInfo. If this condition holds, you have a single block loop. If getLoopFor holds, it's a block somewhere in a multi-block loop. Therefore, this inner condition is stronger than the outer one.
If you only want to process 'relevant' blocks, consider traversing the loop tree rather than the basic block list. In particular, you could limit yourself to the header blocks of the leaf loops, ie. the ones without children.
| unsigned PositionCounter = 0; | ||
|
|
||
| // Collect instruction ordering. | ||
| for (auto &I : B) |
There was a problem hiding this comment.
How about making PositionCounter local-scoped by declaring it here as const unsigned PositionCounter = InstrOrder.size();
| #include "llvm/IR/LLVMContext.h" | ||
| #include "llvm/InitializePasses.h" | ||
| #include "llvm/Support/Debug.h" | ||
| #include "llvm/Transforms/Utils/Cloning.h" |
There was a problem hiding this comment.
I think we only need a fraction of these includes. And as remarked, I think it could be independent on LoopInfo. If you want to generalize later, I would already make the 'is part of the body' test based on comparing getLoopFor of both blocks for equality.
|
I feel that this patch is about pushing the coalescing (or reg alloc) towards a particular direction. |
Hi @qcolombet, indeed it is. This PR was more for discussions, because the effect is very hard to identify. We had one benchmark where the |
|
Thanks @andcarminati for the confirmation! |
This pass helps Greedy to avoid spills.
This pass is indented to solve spill problems related to
-fno-unroll-loops. No specific tests yet, just for evaluation and comments.