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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri 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,
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73705