[PATCH] D86050: [SCEV] Refactor isHighCostExpansionHelper
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 17 06:43:59 PDT 2020
lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.
Please separate SCEV changes into preparatory review.
================
Comment at: llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h:92
+ op_range operands() const {
+ return make_range(&Op, &Op);
+ }
----------------
Doesn't this return empty range?
================
Comment at: llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h:276
- const SCEV *LHS;
- const SCEV *RHS;
+ SmallVector<const SCEV*, 2> Operands;
----------------
`static_assert(sizeof(SmallVector<const SCEV*, 2>) == 4*sizeof(SCEV*), "");` holds, so this is a major bloat.
This should be `std::array<>`.
================
Comment at: llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h:44
+/// given SCEV when expanded into IR.
+struct SCEVOperand {
+ explicit SCEVOperand(unsigned Opc, int Idx, const SCEV *S) :
----------------
The variables could use doxygen comments
================
Comment at: llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h:206
return true; // by always claiming to be high-cost.
- SmallVector<const SCEV *, 8> Worklist;
+ SmallVector<std::unique_ptr<SCEVOperand>, 8> Worklist;
SmallPtrSet<const SCEV *, 8> Processed;
----------------
Why `std::unique_ptr`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86050/new/
https://reviews.llvm.org/D86050
More information about the llvm-commits
mailing list