[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