[cfe-dev] Evaluate constant condition of if statement has no effect

[bluechristlove via cfe-dev]

This is not specific example. If we don’t fix it, user will feel strange why LHS of if stmt condition could do constant folding but RHS couldn’t.

From the source code view’s point, this is our compiler bug. 

In the ExprConstant.cpp:

    case Job::BinOpKind: {
      ...
      if (!VisitBinOpLHSOnly(Result, Bop, SuppressRHSDiags)) {
        Queue.pop_back();
        return;
      }
      ...
      job.Kind = Job::BinOpVisitedLHSKind;
      enqueue(Bop->getRHS());
      return;
    }
      
    case Job::BinOpVisitedLHSKind: {
      ...
      Result.Failed = !VisitBinOp(job.LHSResult, RHS, Bop, Result.Val);
      Queue.pop_back();
      return;
    }

If we can not evaluate LHS successfully, we will evaluate RHS (function VisitBinOp). In the function VisitBinOp:

  if (E->isLogicalOp()) {
    bool lhsResult, rhsResult;
    bool LHSIsOK = HandleConversionToBool(LHSResult.Val, lhsResult);
    bool RHSIsOK = HandleConversionToBool(RHSResult.Val, rhsResult);
    if (LHSIsOK) {
      if (RHSIsOK) {
        if (E->getOpcode() == BO_LOr)
          return Success(lhsResult || rhsResult, E, Result);
        else
          return Success(lhsResult && rhsResult, E, Result);
      }
    } else {
      if (RHSIsOK) {
        // We can't evaluate the LHS; however, sometimes the result
        // is determined by the RHS: X && 0 -> 0, X || 1 -> 1.
        if (rhsResult == (E->getOpcode() == BO_LOr)) {
          return Success(rhsResult, E, Result);
        }
      }
    }

We can see that our previous implementation has consider this condition of RHS constant folding. But because of ignore the parameter Expr::SE_AllowSideEffects in the function ConstantFoldsToSimpleInteger:

bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond,
                                                   llvm::APSInt &ResultInt,
                                                   bool AllowLabels) {
  // FIXME: Rename and handle conversion of other evaluatable things
  // to bool.
  llvm::APSInt Int;
  if (!Cond->EvaluateAsInt(Int, getContext(), Expr::SE_AllowSideEffects)) -------> allow side effect
    return false;  // Not foldable, not integer or not fully evaluatable.

We will not make the RHS constant folding happened.

bool DataRecursiveIntBinOpEvaluator::
       VisitBinOpLHSOnly(EvalResult &LHSResult, const BinaryOperator *E,
                         bool &SuppressRHSDiags) {
….

  if (E->isLogicalOp()) {
    bool LHSAsBool;
    E->getLHS()->dumpColor();
    if (!LHSResult.Failed && HandleConversionToBool(LHSResult.Val, LHSAsBool)) {
      // We were able to evaluate the LHS, see if we can get away with not
      // evaluating the RHS: 0 && X -> 0, 1 || X -> 1
      if (LHSAsBool == (E->getOpcode() == BO_LOr)) {
        Success(LHSAsBool, E, LHSResult.Val);
        return false; // Ignore RHS
      }
    } else {
      LHSResult.Failed = true;

     // Since we weren't able to evaluate the left hand side, it
      // might have had side effects.
      if (!Info.noteSideEffect()) { --------> HERE!!!
        return false;
      }
The detail source code analysis could be find in the comment 2 of https://bugs.llvm.org/show_bug.cgi?id=34229

So, I think we should fix it. 


发件人： John McCall
发送时间： 2017-08-23 01:30:45
收件人：  Frozen
抄送：  cfe-dev at lists.llvm.org
主题： Re: [cfe-dev] Evaluate constant condition of if statement has no effect
On Aug 21, 2017, at 12:28 AM, Frozen via cfe-dev <cfe-dev at lists.llvm.org> wrote:
Simple Example:

int main()
{
  int x;
  if (x || 1) {}
}

Clang can not evaluate this condition and will emit IR like this:

define signext i32 @main() #0 {
entry:
  %retval = alloca i32, align 4
  %x = alloca i32, align 4
  store i32 0, i32* %retval, align 4
  %0 = load i32, i32* %x, align 4
  %tobool = icmp ne i32 %0, 0
  br i1 %tobool, label %if.then, label %lor.lhs.false

lor.lhs.false:                                    ; preds = %entry
  br i1 true, label %if.then, label %if.end

if.then:                                          ; preds = %lor.lhs.false, %entry
  br label %if.end

if.end:                                           ; preds = %if.then, %lor.lhs.false
  %1 = load i32, i32* %retval, align 4
  ret i32 %1
}

However, when we swap the position of LHS and RHS(i.e. if (1 || x), Clang can recognize it:

define signext i32 @main() #0 {
entry:
  %x = alloca i32, align 4
  ret i32 0
}

I also find the root issue and propose one potential solution in comment 2 of this link: https://bugs.llvm.org/show_bug.cgi?id=34229

Any idea?

There will always be some example of something that we generate less efficiently at -O0 than we could.  Why is this example specifically worth optimizing?

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170824/b0dd6da0/attachment.html>