[PATCH] D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost.

[Caroline via Phabricator via llvm-commits]

CarolineConcatto added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h:247
     }
-    assert(BudgetRemaining >= 0 && "Should have returned from inner loop.");
+    assert(BudgetRemaining.isValid() && "Expected a valid cost");
+    assert(*BudgetRemaining.getValue() >= 0 &&
----------------
Maybe put this assertion before the loop? Is there any reason to check the Cost if BudgetRemaining.isValid()  is not valid?
Or maybe you can remove this assert from here, because if we check isHighCostExpansionHelper this test is done in line 2346.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:2208
   const T *S = cast<T>(WorkItem.S);
   int Cost = 0;
   // Object to help map SCEV operands to expanded IR instructions.
----------------
Why you did not changed Cost to be InstructionCost?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92238