[PATCH] D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains

Yatsina, Marina via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 2 11:11:47 PDT 2017


Hi Quentin,

Let’s currently focus on the “bad eviction” patch which is under review, as the other issue (“spill cost of local interval”) is still half-baked and will probably need more refinement ☺

The compilation time measurements I mentioned refer to the time of the whole compilation.
I’ve ran the measurements a couple of times again and I see that my compilation times are not consistent.
For example, a workload that compiles ~4  minutes - at one run compiles 30% worse with my patch and at another run compiles 5% better with my patch.
Workloads that have short compile time (10-20 seconds) also show such big variations.

The performance gains are consistent and I see them both for workloads that run 10 seconds and for workloads that run 5 minutes.
* The gains and the size of the gain depend very much on the revision you are checking – you may see a very nice gain at revision X, but if at revision X+1 somebody changed something in an earlier pass which affects the llvm ir/machine ir, it may change the register allocation and now these bad eviction chains don’t happen anymore for this workload, so this patch will no longer show gains.

The additional prediction I’ve added only happens if the candidate will create local intervals and only for those intervals.
The check itself of whether a local interval will be created does not add additional complexity.
If the compile time is still a concern - can you check the compile on your systems and let me know if you see significant issues with my patch?

Also, do you have any comments regarding the patch itself?

Thanks,
Marina


From: qcolombet at apple.com [mailto:qcolombet at apple.com]
Sent: Thursday, September 28, 2017 19:45
To: Yatsina, Marina <marina.yatsina at intel.com>
Cc: reviews+D35816+public+4cab5a74a0b7bc06 at reviews.llvm.org; stoklund at 2pi.dk; Matthias Braun <matze at braunis.de>; llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [PATCH] D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains

Hi Maria,

Thanks for the numbers.

Couple more questions.




On Sep 27, 2017, at 4:19 PM, Yatsina, Marina <marina.yatsina at intel.com<mailto:marina.yatsina at intel.com>> wrote:

According to my measurement the patch in the review (considering if the local interval might cause "bad" eviction) gives to 2-13% gain in eembc, coremark-pro and geekbench.
I have a regression of about 2.5% which seem to be related to different decisions taken later down the road because now the allocation is changed. It seems to expose another not optimal decision of the register allocator and probably worth investigating, but unrelated to the problem I'm trying to solve now.
compile time - improvement of up to 17.4x, regression up to 9.5x, geomean - 1.8% more compile time.


Is this the pass itself of the whole compiler?

Also, could you share the raw data, if we are talking minutes in one case and seconds in the other case (and vice-versa) this is not really comparable.



The initial patch I have for considering if the local interval might spill gives up to 50% gain.
Here I have a few regressions of 2-4.5% which I'm investigating.

Ditto, raw numbers would be helpful.


compile time - improvement of up to 8x, regression up to 6.8x, geomean - 14% more compile time.

When combining both patches: gains of up to 50%, regression of up to 5.24%.



compile time - improvement of up to 6x, regression up to 5.7x, geomean - 4.3% more compile time.


Ditto and same question about pass vs. whole compile time.



I will try to improve the compile time for the spill consideration, this should also positively affect the compile time of the combination of the 2 patches together.

Thanks,
-Quentin



Thanks,
Marina

From: qcolombet at apple.com<mailto:qcolombet at apple.com> [mailto:qcolombet at apple.com]
Sent: Monday, September 25, 2017 23:10
To: Yatsina, Marina <marina.yatsina at intel.com<mailto:marina.yatsina at intel.com>>
Cc: reviews+D35816+public+4cab5a74a0b7bc06 at reviews.llvm.org<mailto:reviews+D35816+public+4cab5a74a0b7bc06 at reviews.llvm.org>; stoklund at 2pi.dk<mailto:stoklund at 2pi.dk>; Matthias Braun <matze at braunis.de<mailto:matze at braunis.de>>; llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>>; Quentin Colombet <qcolombet at apple.com<mailto:qcolombet at apple.com>>
Subject: Re: [PATCH] D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains





On Sep 25, 2017, at 10:44 AM, Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:


On Sep 25, 2017, at 8:46 AM, Marina Yatsina via Phabricator <reviews at reviews.llvm.org<mailto:reviews at reviews.llvm.org>> wrote:

myatsina added a comment.

Hi Quentin,

I wouldn’t say my patch tries to avoid splitting, but rather tries to improve the calculation of the spill weight of split candidates:
When the register allocator decides to do a region split, it looks for the best physical register candidate for the split.
The best candidate is the one that will cause the minimal spill cost.
When calculating the spill cost of each candidate the algorithm takes into account interferences in the entrance/exist of the basic blocks.
However, there may be interference local to a basic block, which later, during the split itself will cause the creation of a new local interval (which will be local to the basic block) on top of the “by reg” and “by stack” intervals which are created during the split.
The algorithm currently ignores the fact that this local interval may cause spills (and thus may increase the spill weight of this candidate for the split).

My solution is to try to predict if this split candidate will case the creation of local intervals and if they in turn will cause spills, and add their spill weights to the total weight.
By doing so, I try to make the spill weight calculation of each candidate more accurate and allow the algorithm to choose a more suitable candidate.

If a local interval is created then we have a few options for its allocation:

1. The interval will be allocated to some free reg – no additional spill cost needed.
2. The interval may cause an eviction – in some cases this eviction is "bad" and guaranteed to causes a spill (it’s “bad” when you’re evicting the interval that evicted you, kind of like a cat and mouse game - somebody must loose here) - in this patch I’m trying to predict if it’s "bad" or not, and incorporate the spill weight of this interval.
3. The interval may spill – I’ve already encountered a case where the new local interval is in a hot loop and ends us spilling around all uses – this spill cost wasn’t considered when the candidate was chosen. I have a solution for this case which is based on parts of this patch.
4. The interval may split – I guess there might be some spill cost to consider here as well, but I didn’t explore this case yet.

I did see nice performance results with my current solution.

I can totally see that. I wonder if adding 1K LOC is worth it and in particular if we can’t have the same result with fewer LOC.
My main concern is we try to predict yet another thing. Again the approach itself as you describe it makes sense, but the trade off complexity reward is not clear to me. In particular, what is the compile time impact, what are the regressions and so on.

Assuming this all looks good, I’ll give the patch another look, so please share your numbers.






I will try to look into the hint reconciling as well, but I do think that the current spill weight calculation of the split candidates is not accurate enough and we need to consider the affects of those local intervals.

Thanks,
Marina


Repository:
rL LLVM

https://reviews.llvm.org/D35816




_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171002/231c86f7/attachment.html>


More information about the llvm-commits mailing list