[PATCH] D142803: [LogicCombine 1/?] Implement a general way to simplify logical operations.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

> I need LeafValues to access value by index. It looks SmallSetVector can't do that.

SmallSetVector allows indexing. This is the patch I tried after applying this patch (I had to fix line endings first), and no tests fail:

  diff --git a/llvm/include/llvm/Analysis/LogicCombine.h b/llvm/include/llvm/Analysis/LogicCombine.h
  index 3fdcf7998321..56a3d8f36b16 100644
  --- a/llvm/include/llvm/Analysis/LogicCombine.h
  +++ b/llvm/include/llvm/Analysis/LogicCombine.h
  @@ -8,8 +8,7 @@
  
   #include "LogicalExpr.h"
   #include "llvm/ADT/DenseMap.h"
  -#include "llvm/ADT/SmallPtrSet.h"
  -#include "llvm/ADT/SmallVector.h"
  +#include "llvm/ADT/SetVector.h"
   #include "llvm/ADT/Statistic.h"
   #include "llvm/IR/InstrTypes.h"
   #include "llvm/IR/Instruction.h"
  @@ -50,8 +49,7 @@ private:
     friend class LogicalOpNode;
  
     SmallDenseMap<Value *, LogicalOpNode *, 16> LogicalOpNodes;
  -  SmallPtrSet<Value *, 8> LeafSet;
  -  SmallVector<Value *, 8> LeafValues;
  +  SmallSetVector<Value *, 8> LeafValues;
  
     void clear();
  
  diff --git a/llvm/lib/Analysis/LogicCombine.cpp b/llvm/lib/Analysis/LogicCombine.cpp
  index 28d9488cab96..3b410cdacd32 100644
  --- a/llvm/lib/Analysis/LogicCombine.cpp
  +++ b/llvm/lib/Analysis/LogicCombine.cpp
  @@ -101,17 +101,16 @@ void LogicCombiner::clear() {
     for (auto node : LogicalOpNodes)
       delete node.second;
     LogicalOpNodes.clear();
  -  LeafSet.clear();
     LeafValues.clear();
   }
  
   LogicalOpNode *LogicCombiner::visitLeafNode(Value *Val, unsigned Depth) {
     // Depth is 0 means the root is not logical operation. We can't
     // do anything for that.
  -  if (Depth == 0 || LeafSet.size() >= MaxLogicOpLeafsToScan)
  +  if (Depth == 0 || LeafValues.size() >= MaxLogicOpLeafsToScan)
       return nullptr;
  
  -  uint64_t ExprVal = 1ULL << LeafSet.size();
  +  uint64_t ExprVal = 1ULL << LeafValues.size();
     // Constant Zero,AllOne are special leaf nodes. They involve
     // LogicalExpr's calculation so we must detect them at first.
     if (auto ConstVal = dyn_cast<ConstantInt>(Val)) {
  @@ -120,9 +119,8 @@ LogicalOpNode *LogicCombiner::visitLeafNode(Value *Val, unsigned Depth) {
       else if (ConstVal->isAllOnesValue())
         ExprVal = LogicalExpr::ExprAllOne;
     }
  -  if (ExprVal != LogicalExpr::ExprAllOne && ExprVal != 0 &&
  -      LeafSet.insert(Val).second)
  -    LeafValues.push_back(Val);
  +  if (ExprVal != LogicalExpr::ExprAllOne && ExprVal != 0)
  +    LeafValues.insert(Val);
     LogicalOpNode *Node = new LogicalOpNode(this, Val, LogicalExpr(ExprVal));
     LogicalOpNodes[Val] = Node;
     return Node;


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

https://reviews.llvm.org/D142803