[PATCH] D116688: [SLP]Excluded external uses from the reprdering estimation.

[Vasileios Porpodas via Phabricator via llvm-commits]

vporpo added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1189
+      Value *OpIdxLaneV = getData(OpIdx, Lane).V;
+      if (isVectorLikeInstWithConstOps(IdxLaneV) &&
+          isVectorLikeInstWithConstOps(OpIdxLaneV))
----------------
Could you add a comment explaining why we return `ScoreAllUserVectorized` for this type of instructions?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1364
           Value *OpRight = (LeftToRight) ? Op : OpLastLane;
-          unsigned Score =
-              getLookAheadScore({OpLeft, LastLane}, {OpRight, Lane});
-          if (Score > BestOp.Score) {
+          int Score = getLookAheadScore(OpLeft, OpRight, MainAltOps);
+          if (Score) {
----------------
Since the score calculation is a bit more complicated now, I think it makes sense to move all the score calculation logic into a separate function like `getScore()` which will help hide all the calls to the separate score functions `getLookAheadScore()`, `getSplatScore()`, `getExternalScore()` and the score scaling. What do you think?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1378
+              // operand with all vectorized uses.
+              Score *= 10;
+              Score += getExternalUseScore(Lane, OpIdx, Idx);
----------------
nit: use a static constexpr for the scaling factor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116688