-
-
Notifications
You must be signed in to change notification settings - Fork 668
Fix and enable branch minimization in backend #22158
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
base: master
Are you sure you want to change the base?
Conversation
|
I don't understand quite what this does, but I like the results! (Why is it still in progress?) |
|
Hey, you wrote the original code, I expected there to be some story around this. It rearranges basic blocks to reduce jumps. For example, a catch block in the middle of a function can be reordered to after the epilogue, so the locality is better. It still has some issues with loops in the current form. while (true)
{
if (f())
{
g();
break;
}
else
h();
}
static foreach (i; 0 .. 64) a[i] = i;
return;The else block will be moved to the end of the function despite the long distance, which gives atrocious locality to the loop. Lstart:
while (true)
{
if (f())
{
g();
break;
}
else
goto Lend;
}
static foreach (i; 0 .. 64) a[i] = i;
return;
Lend:
h();
goto Lstart;Statically it's impossible to determine the hot branch at runtime, so such motion is unacceptable. I'm considering limiting the distance of motion, or just inhibiting motion inside a loop to fix this. I wonder if there are better ways to do this. |
|
We did this for the assert failure branches sometime ago of moving them past the function epilogue. |
|
Thanks for your pull request and interest in making D better, @limepoutine! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#22158" |
|
Added a completely nonsensical cost model to prevent pathological cases. Anyone has better ideas? |
There is an incomplete jump minimization pass in the backend, that is never used (since the first SVN revision of D2). This PR modernizes and enables the pass. The algorithm is still quadratic, but pathological case is somewhat rare because of restrictions from the glue layer. I took some care not to reorder if-chains to avoid disrupting branch prediction.
Generally, jump minimization reduces frontend stall and provides 5% improvement for branchy code. There is 2% improvement for Jabba Laci's AoC program and very tiny improvement for code in this SO thread (compiled with
-inline -O -release, tested withhyperfine -w 3 -r 15, on an Intel i5-12500H with atom cores disabled):Let's see if it breaks anything.