[PATCH] D94069: [NFC][InstructionCost]Migrate VectorCombine.cpp to use InstructionCost

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:200
   // invert this transform if it does not result in a performance win.
+  assert(OldCost.isValid() && "Invalid initial cost");
   if (OldCost < NewCost)
----------------
Is this needed?

- If the old cost is invalid, but the new cost is valid, then do the transform.
- If neither cost is valid, then `OldCost < NewCost` will cause a return of false.




================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:250
 
+  assert((Cost0.isValid() || Cost1.isValid()) &&
+         "At least one of Cost0 or Cost1 should be valid");
----------------
Reasonable to return nullptr?

If neither cost is valid, then neither of the inputs should be replaced


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:358-359
 
+  assert((OldCost.isValid() || NewCost.isValid()) &&
+         "At least one of OldCost or NewCost should be valid");
   // Aggressively form a vector op if the cost is equal because the transform
----------------
Would it be reasonable to return false here? If all costs involved are invalid, then I would say the transform is not cheap.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:526
+      TTI.getShuffleCost(TargetTransformInfo::SK_PermuteSingleSrc, SrcTy);
+  assert(SrcCost.isValid() && "Invalid source cost");
+  if (DestCost > SrcCost)
----------------
reasonable to return false here? If neither cost is valid, then do not do the transform


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:637
   // We want to scalarize unless the vector variant actually has lower cost.
+  assert(OldCost.isValid() && "Invalid initial cost");
   if (OldCost < NewCost)
----------------
reasonable to return false?


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:739
   // Codegen can reverse this transform (scalarize) if it was not profitable.
+  assert(OldCost.isValid() && "Invalid initial cost");
   if (OldCost < NewCost)
----------------
reasonable to return false?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94069