Page MenuHomePhabricator

[NFC][regalloc] Make VirtRegAuxInfo part of allocator state
ClosedPublic

Authored by mtrofin on Sep 29 2020, 9:11 AM.

Details

Summary

All the state of VRAI is allocator-wide, so we can avoid creating it every time we need it, to simplify readability - the way weights are calculated is decided per algorithm. In addition, the normalization function is allocator-specific. In a next change, we can simplify that design in favor of just having it as a virtual member. This also enables experimenting with alternative weight calculation mechanisms, where the decision on which to use is made in one place (i.e. where the VRAI is instantiated).

Diff Detail

Event Timeline

mtrofin created this revision.Sep 29 2020, 9:11 AM
mtrofin requested review of this revision.Sep 29 2020, 9:11 AM
qcolombet requested changes to this revision.Sep 29 2020, 10:13 AM

Hi,

From the perspective of the caller, this patch exposes some implementation details that I am not sure are worth it.

Put differently, what is the expected benefits of moving VRAI life-time to the allocators?

VRAI are relatively light-weight objects and I wouldn't expect saving on the constructor to bring us any gain. So what are we after here?

Cheers,
-Quentin

This revision now requires changes to proceed.Sep 29 2020, 10:13 AM

Hi,

From the perspective of the caller, this patch exposes some implementation details that I am not sure are worth it.

Put differently, what is the expected benefits of moving VRAI life-time to the allocators?

VRAI are relatively light-weight objects and I wouldn't expect saving on the constructor to bring us any gain. So what are we after here?

Cheers,
-Quentin

Indeed, it's not a performance issue, I see the motivation may read that way. It's helping with readability around weight calculation. The patch clarifies that the way we calculate weights is a per-allocation algorithm concern - including which normalization function we use. When looking at Greedy, for example, it's easier to read that "weight calculation always happens the same way". My next step would be to then make the normalization a virtual, and have a PBQPVirtRegAuxInfo overriding it.

Finally, as another next step, it simplifies exploring other weight calculation strategies: we make weightCalcHelper virtual, and go from there. Same reasoning as above - in Greedy, for instance, we can then just instantiate the desired VirtRegAuxInfo variant in the one place, rather than chasing all instantiation sites.

mtrofin edited the summary of this revision. (Show Details)Sep 29 2020, 10:42 AM
qcolombet accepted this revision.Sep 29 2020, 2:30 PM

When looking at Greedy, for example, it's easier to read that "weight calculation always happens the same way".

I see. The readability improvement is worth it, I agree.

This revision is now accepted and ready to land.Sep 29 2020, 2:30 PM