[PATCH] D15302: [Greedy regalloc] Replace analyzeSiblingValues with something new [Part1]

[Quentin Colombet via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added a comment.
This revision now requires changes to proceed.

Hi Wei,

I had a quick look at the patch and although I believe it does the job done, I do not think this is the way to fix the problem.
My understanding is that you are trading an expensive value tracking mechanism with a fancy, but apparently less expensive, spill placement mechanism.

I think it is not the right approach because this creates a (bigger) gap between the cost model of the register allocator and the actual spill cost. Indeed, the cost model of the register allocator, w.r.t. spill cost, is basically reload before the uses, spill after the definitions.
In other word, it is better to keep the spiller simple but be smarter on the splitting of live-ranges so that the register allocator takes the right spilling/splitting decisions. That way, sharing spills/reloads will come naturally without to do anything in the spiller plus we may get better copies placement.

This was also, I believe, what Jakob had in mind when he described a solution in PR17409.

Concretely, what you should do:
0. Create a baseline for performance comparisons without any changes

1. Add an option to disable the InlineSpiller::analyzeSiblingValues, say -disable-spill-analyze-sibvalue
2. Benchmark with (-mllvm) -disable-spill-analyze-sibvalue  (-mllvm) -split-spill-mode=size (we may want to use “speed" and improve that splitting mode instead of “size")
3. Investigate the regressions and/or file PRs
4. Fix the regressions

At this point, the new (or size) split mode should be better or equivalent to the baseline and we can just kill analyzeSiblingValue and make that new mode the default.

Hope that helps.

Cheers,
-Quentin


Repository:
  rL LLVM

http://reviews.llvm.org/D15302