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

[Richard Smith via cfe-dev]

On 25 Aug 2017 10:27, "bluechristlove via cfe-dev" <cfe-dev at lists.llvm.org>
wrote:

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.


1 || x is a constant expression.
x || 1 is not.
This is a property of the underlying programming language -- || short
circuits. The user needs to understand that.

>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 <rjmccall at apple.com>
*发送时间：* 2017-08-23 01:30:45
*收件人：*  Frozen <bluechristlove at 163.com>
*抄送：*  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.


_______________________________________________
cfe-dev mailing list
cfe-dev at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170825/5662fa0a/attachment.html>