[PATCH] D73705: [NFC][SCEV] Piping to pass new SCEVCheapExpansionBudget option into SCEVExpander::isHighCostExpansionHelper()

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ScalarEvolutionExpander.h:331
     bool isHighCostExpansionHelper(const SCEV *S, Loop *L,
-                                   const Instruction *At,
+                                   const Instruction *At, int &BudgetRemaining,
                                    const TargetTransformInfo *TTI,
----------------
lebedev.ri wrote:
> mkazantsev wrote:
> > Please make all budgets either signed or unsigned. This should be consistent.
> There are two functions here: `isHighCostExpansionHelper()`
> and `isHighCostExpansion()`.
> 
> The helper is internal and self-recursive, and must have *signed*
> budget parameter since it makes logic much better.
> 
> The `isHighCostExpansion()` is public, and is normally called with
> just the `SCEVCheapExpansionBudget` param. I'm not sure it makes
> much sense in accepting that param as signed,
> because what would negative budget mean?
> 
> I suppose i could rename `scev-cheap-expansion-budget` param
> to `scev-cheap-expansion-threshold` so all budgets are signed,
> if that is better?
And how it's going to work if the user specifies bugdet greater than max signed int? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73705