[PATCH] D37198: [InlineCost] add visitSelectInst()

[Chad Rosier via Phabricator via llvm-commits]

mcrosier added a comment.

Overall, the logic of the patch is in good shape.  However, I'd suggest some minor refactoring to delineate the select of constants/values vs. GEPs for SROA.

Maybe something like this:

  bool CallAnalyzer::visitSelectInst(SelectInst &SI) {
    Value *TrueVal = SI.getTrueValue();
    Value *FalseVal = SI.getFalseValue();
  
    Constant *TrueC = isa<Constant>(TrueVal) ? cast<Constant>(TrueVal)
                                             : SimplifiedValues.lookup(TrueVal);
    Constant *FalseC = isa<Constant>(FalseVal)
                           ? cast<Constant>(FalseVal)
                           : SimplifiedValues.lookup(FalseVal);
    // Select C, X, X => X
    if (TrueC && FalseC && TrueC == FalseC) {
      SimplifiedValues[&SI] = TrueC;
      return true;
    }
  
    Type *ValTy = FalseVal->getType();
    assert(TrueVal->getType() == FalseVal->getType() &&
           "Expected matching types.");
  
    Constant *CondC =
        dyn_cast_or_null<Constant>(SimplifiedValues.lookup(SI.getCondition()));
    // Select condition is a constant.
    if (CondC && !ValTy->isPointerTy()) {
      // Select True, X, Y => X
      if (CondC->isAllOnesValue() && TrueC)
        SimplifiedValues[&SI] = TrueC;
      // Select False, X, Y => Y
      if (CondC->isNullValue() && FalseC)
        SimplifiedValues[&SI] = FalseC;
      // If all operands are constants.  ConstantExpr::getSelect() can handle
      // rest cases such as select vectors.
      if (TrueC && FalseC)
        if (Constant *C = ConstantExpr::getSelect(CondC, TrueC, FalseC))
          SimplifiedValues[&SI] = C;
      return true;
    }
  
    // Logic for SROA follows
    // Select C, X, X => X
    // Select True, X, Y => X
    // Select False, X, Y => Y
  
    return Base::visitSelectInst(SI);
  }

IMHO this is a little easier to read.


Repository:
  rL LLVM

https://reviews.llvm.org/D37198