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

[Vasileios Porpodas via Phabricator via llvm-commits]

vporpo added a comment.

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.
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.

Another point is that if we need to avoid this extra traversal, we could perhaps implement this within the scheduler. It already walks through all the blocks while building the dependence graph. For example we could update the LiveValues in tryScheduleBundle() and compute the cost over calls in some function like extendSchedulingRegion() ?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64523