[PATCH] D95351: NFC: Migrate SimplifyCFG to work on InstructionCost

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:338
 /// expensive.
-static unsigned ComputeSpeculationCost(const User *I,
-                                       const TargetTransformInfo &TTI) {
+static InstructionCost ComputeSpeculationCost(const User *I,
+                                              const TargetTransformInfo &TTI) {
----------------
spatel wrote:
> Here and below: since we're updating function signatures, it would make sense to update the formatting as clang-tidy suggests:
> computeSpeculationCost()
> 
> This can be done as an NFC preliminary commit.
That's a great suggestion, thanks!


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:364
                                 SmallPtrSetImpl<Instruction *> &AggressiveInsts,
-                                int &BudgetRemaining,
+                                InstructionBudget &BudgetRemaining,
                                 const TargetTransformInfo &TTI,
----------------
spatel wrote:
> Which patch defines `InstructionBudget`?
This was initially added in D92238, but after some discussion on the patch I removed it in favour of using an incrementing Cost value that compares directly with a `Budget` value.


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

https://reviews.llvm.org/D95351