-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CodeGen] Preserve LiveStack analysis in StackSlotColoring pass #94967
[CodeGen] Preserve LiveStack analysis in StackSlotColoring pass #94967
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
6a69b4d
to
e13e0f8
Compare
For some target architectures, stack slot coloring pass may be invoked multiple times. It can occur due to the split of RA pass, opening up the opportunity to optimize stack slots usage at each RA invocation. In order to achieve that if we could keep stack analysis alive uptil the invocation of the RA pass for the last time, it will save up the overhead of recomputing live stack analysis after each RA. This requires it to be preserved at stack slot coloring pass which basically optimizes stack slots, but not makes the needed updates in live stack results. This has been achieved here.
@llvm/pr-subscribers-llvm-regalloc Author: Vikash Gupta (vg0204) ChangesThis PR originated as a follow-up requirement from #93668. For some target architectures, stack slot coloring pass may be invoked multiple times. It can occur due to the split of RA pass, opening up the opportunity to optimize stack slots usage after each RA invocation. In order to achieve that if we could keep livestack analysis alive uptil the invocation of the RA pass for the last time in pipeline, it would save up the overhead of recomputing live stack analysis after each RA. Thus, livestack result requires it to be preserved at stack slot coloring pass which basically optimizes stack slots, but not makes the needed updates in live stack results. This has been achieved here in this patch. Full diff: https://github.com/llvm/llvm-project/pull/94967.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/StackSlotColoring.cpp b/llvm/lib/CodeGen/StackSlotColoring.cpp
index 9fdc8a338b52a..7bf966b21ebc1 100644
--- a/llvm/lib/CodeGen/StackSlotColoring.cpp
+++ b/llvm/lib/CodeGen/StackSlotColoring.cpp
@@ -149,6 +149,7 @@ namespace {
AU.addRequired<SlotIndexes>();
AU.addPreserved<SlotIndexes>();
AU.addRequired<LiveStacks>();
+ AU.addPreserved<LiveStacks>();
AU.addRequired<MachineBlockFrequencyInfo>();
AU.addPreserved<MachineBlockFrequencyInfo>();
AU.addPreservedID(MachineDominatorsID);
@@ -411,6 +412,23 @@ bool StackSlotColoring::ColorSlots(MachineFunction &MF) {
}
}
+ // In order to preserve LiveStack analysis, the live ranges for dead spill
+ // stack slots would be merged with the live range of those stack slots that
+ // now share the spill object of the mentioned dead stack slot.
+ for (unsigned SS = 0, SE = SlotMapping.size(); SS != SE; ++SS) {
+ int NewFI = SlotMapping[SS];
+ if (SlotMapping[SS] == -1 || (NewFI == (int)SS)) {
+ continue;
+ }
+
+ LiveRange &lrToUpdateInto =
+ static_cast<LiveRange &>(LS->getInterval(NewFI));
+ const LiveRange &lrToUpdateFrom =
+ static_cast<LiveRange &>(LS->getInterval((int)SS));
+ lrToUpdateInto.MergeSegmentsInAsValue(lrToUpdateFrom,
+ lrToUpdateInto.getValNumInfo(0));
+ }
+
return true;
}
|
} | ||
|
||
LiveRange &lrToUpdateInto = | ||
static_cast<LiveRange &>(LS->getInterval(NewFI)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't need this cast?
lrToUpdateInto.MergeSegmentsInAsValue(lrToUpdateFrom, | ||
lrToUpdateInto.getValNumInfo(0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use join
.
This implementation is not as straightforward as it seems, as my current solution does not cover lot of corner cases. Also, I found that eventhough not stack slot re-usage may occur but stack slot coloring effectively re-assigns stack slots, which is currently like any change not updated in live stack. As a result, for example if a piece of code has 100 Stack slots objects all of which are just being re-assigned to new SS index supposedly (with no SS index sharing possible), this has to be propagated in LS results, which I feel is equivalent to creating entire new LS. Besides if there occur stack slot share, those all need to collected (could be any number), join their live ranges as well map them to newly assigned stack slot index, as it is very possible that all stack slots being shared themselves might get SS index re-assigned. |
So, considering all this I am just refelcting whether its worthwhile to preserve liveStacks, as once used by SCC in middle, it may be of no use to later upcoming RA passes(as they themselves enter their own stack spills livenes to be later maybe used by SCC). So one probable solution in my suggestion could be reset the LS results. |
// efforts to rectify which is equivalent to do it all over again. Also, | ||
// this current results would not be used later, so better to clear it & | ||
// preserve analysis. | ||
LS->releaseMemory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're just going to throw away the result there's no point in claiming this is preserved, since it's a lie
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, currently LiveStack analysis really is doing nothing just creating a empty shell result, & real work is happening at RA to compute and update livestack result. Also, once generated at RA pass, spill entries in LS will later be used at stackSlotColoring pass, followed by not being used ever again (LS later be used during SCC pass coming after later RA passes, but at that point RA is concerned with newly introduced spill records in LS by itself). So, my point is that preserving LS won't help out in any way. And LS analyis itself is doing no computation, so its re-occurence should not have impact on performance.
Should close this then? |
Yes, It should be as It truly does not makes thing effective anyway. |
This PR originated as a follow-up requirement from #93668.
For some target architectures, stack slot coloring pass may be invoked multiple times. It can occur due to the split of RA pass, opening up the opportunity to optimize stack slots usage after each RA invocation.
In order to achieve that if we could keep livestack analysis alive uptil the invocation of the RA pass for the last time in pipeline, it would save up the overhead of recomputing live stack analysis after each RA. Thus, livestack result requires it to be preserved at stack slot coloring pass which basically optimizes stack slots, but not makes the needed updates in live stack results. This has been achieved here in this patch.