[PATCH] D64523: [SLPVectorizer] Fix getSpillCost() calculation

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D64523#1583624 <https://reviews.llvm.org/D64523#1583624>, @vporpo wrote:

> I guess it would be better if it visited all the BBs. You could traverse the VectorizableTree once and collect the BBs to be visited before you walk through them. Btw the TreeEntries now have operands, which can help you collect the BBs in the right order.


I'm not familiar with the SLP vectorizer, is there a guarantee that the BBs form a chain of dominating BBs? I.e. is there always a single BB we can go to when the edge of the current one is reached?

> But I am not sure this is worth the trouble, because it looks to me that the spill cost estimation function is not very accurate in the first place. It only considers the spill cost over calls, and while doing so it is not comparing it against the spill cost of the scalar code, which if I am not mistaken could be equal to that of the vector code in many cases. Someone who knows more about the assumptions made here should probably comment on it.

The spill cost calculation is only used on AArch64. I believe the intention is mainly to avoid some "trivial" vectorizations that would be detrimental because AArch64 has scalar, but not vector, callee-save registers.

The spill cost estimation is pretty inaccurate for multiple reasons. One is the implementation issue I try to address here, another is the fact that intrinsics are assumed to also cause spilling, another is that each call is considered independently, even though spill costs will likely be amortized across multiple calls. And, as you say, spill costs for the scalar code are not modeled.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64523/new/

https://reviews.llvm.org/D64523