[PATCH] D59973: [SLP] Refactoring of the operand reordering code.

[Vasileios Porpodas via Phabricator via llvm-commits]

vporpo marked an inline comment as done.
vporpo added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1010
+                // getBestOperand().
+                swap(OpIdx, BestIdx.getValue(), Lane);
+              else {
----------------
ABataev wrote:
> I think the first pass should not do any swaps.
Well, if we don't do the swap, then getBestOperand() won't work, as it relies on the state of OpsVec for checking the previous lane vs the current one. We could potentially use some other helper structure to avoid updating the state in OpsVec, but that would complicate the code quite a bit.

Also, I don't see any harm in applying the reordering in the first pass. The worst that can happen is that the order in OpsVec will change in the second pass. Am I missing out something here?


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

https://reviews.llvm.org/D59973