<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div>Not. Original implementation used the default parameter value, i.e. No Side Effects<br><div><br></div><div> /// EvaluateAsInt - Return true if this is a constant which we can fold and</div><div> /// convert to an integer, using any crazy technique that we want to.</div><div> bool EvaluateAsInt(llvm::APSInt &Result, const ASTContext &Ctx,</div><div> <b>SideEffectsKind AllowSideEffects = SE_NoSideEffects</b>) const;<br><br>But, now, we should <b>allow side effects</b> and pass the computation to the function <b class="" style="font-family: "PingFang SC"; font-size: 10.5pt;">Info.noteSideEffect(). </b>As the comment of source code said:</div></div><div><div><br></div><div>bool DataRecursiveIntBinOpEvaluator::</div><div> VisitBinOpLHSOnly(EvalResult &LHSResult, const BinaryOperator *E,</div><div> bool &SuppressRHSDiags) {</div><div> if (E->isLogicalOp()) {</div><div> bool LHSAsBool;</div><div> if (!LHSResult.Failed && HandleConversionToBool(LHSResult.Val, LHSAsBool)) {</div><div> // We were able to evaluate the LHS, see if we can get away with not</div><div> // evaluating the RHS: 0 && X -> 0, 1 || X -> 1</div><div> if (LHSAsBool == (E->getOpcode() == BO_LOr)) {</div><div> Success(LHSAsBool, E, LHSResult.Val);</div><div> return false; // Ignore RHS</div><div> }</div><div> } else {</div><div> LHSResult.Failed = true;</div><div><br></div><div><b> // Since we weren't able to evaluate the left hand side, it</b></div><div><b> // might have had side effects.</b></div><div><b> if (!Info.noteSideEffect())</b></div><div><b> return false;</b></div><div><br></div><div> // We can't evaluate the LHS; however, sometimes the result</div><div> // is determined by the RHS: X && 0 -> 0, X || 1 -> 1.</div><div> // Don't ignore RHS and suppress diagnostics from this arm.</div><div> SuppressRHSDiags = true;</div><div> }</div><div><br></div><div> return true;</div><div> }</div></div><div><br></div><div>So we might have had side effects when constant-evaluation in if stmt condition , so we should use <b class="" style="font-family: "PingFang SC"; font-size: 10.5pt;">Expr::SE_AllowSideEffects, </b>then use <b>Info.noteSideEffect() </b>to compute the result. </div><br><div style="position:relative;zoom:1"></div><div id="divNeteaseMailCard"></div><br>在 2017-08-24 02:02:23，"John McCall" <rjmccall@apple.com> 写道：<br> <blockquote id="isReplyContent" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 23, 2017, at 1:58 PM, bluechristlove <<a href="mailto:bluechristlove@163.com" class="">bluechristlove@163.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class=""><div style="font-family: 'PingFang SC'; font-size: 10.5pt;" class="">
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.<div class=""><br class=""></div><div class="">From the source code view’s point, this is our compiler bug.</div></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div class=""><div style="font-family: 'PingFang SC'; font-size: 10.5pt;" class=""><div class=""><br class=""></div><div class="">In the ExprConstant.cpp:</div><div class=""><br class=""></div><div class=""><div class=""> case Job::BinOpKind: {</div><div class=""> ...</div><div class=""> if (!VisitBinOpLHSOnly(Result, Bop, SuppressRHSDiags)) {</div><div class=""> Queue.pop_back();</div><div class=""> return;</div><div class=""> }</div><div class=""> ...</div><div class=""> job.Kind = Job::BinOpVisitedLHSKind;</div><div class=""> enqueue(Bop->getRHS());</div><div class=""> return;</div><div class=""> }</div><div class=""> </div><div class=""> case Job::BinOpVisitedLHSKind: {</div><div class=""> ...</div><div class=""> Result.Failed = !VisitBinOp(job.LHSResult, RHS, Bop, Result.Val);</div><div class=""> Queue.pop_back();</div><div class=""> return;</div><div class=""> }</div><div class=""><br class=""></div><div class="">If we can not evaluate LHS successfully, we will evaluate RHS (function VisitBinOp). In the function VisitBinOp:</div><div class=""><br class=""></div><div class=""><div class=""> if (E->isLogicalOp()) {</div><div class=""> bool lhsResult, rhsResult;</div><div class=""> bool LHSIsOK = HandleConversionToBool(LHSResult.Val, lhsResult);</div><div class=""> bool RHSIsOK = HandleConversionToBool(RHSResult.Val, rhsResult);</div><div class=""> if (LHSIsOK) {</div><div class=""> if (RHSIsOK) {</div><div class=""> if (E->getOpcode() == BO_LOr)</div><div class=""> return Success(lhsResult || rhsResult, E, Result);</div><div class=""> else</div><div class=""> return Success(lhsResult && rhsResult, E, Result);</div><div class=""> }</div><div class=""> } else {</div><div class=""> if (RHSIsOK) {</div><div class=""><b class=""> // We can't evaluate the LHS; however, sometimes the result</b></div><div class=""><b class=""> // is determined by the RHS: X && 0 -> 0, X || 1 -> 1.</b></div><div class=""> if (rhsResult == (E->getOpcode() == BO_LOr)) {</div><div class=""> return Success(rhsResult, E, Result);</div><div class=""> }</div><div class=""> }</div><div class=""> }</div></div><div class=""><br class=""></div><div class="">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 <span style="font-size: 10.5pt;" class="">ConstantFoldsToSimpleInteger</span><span style="font-size: 10.5pt;" class="">:</span></div><div class=""><br class=""></div><div class=""><div class="">bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond,</div><div class=""> llvm::APSInt &ResultInt,</div><div class=""> bool AllowLabels) {</div><div class=""> // FIXME: Rename and handle conversion of other evaluatable things</div><div class=""> // to bool.</div><div class=""> llvm::APSInt Int;</div><div class=""> if (!Cond->EvaluateAsInt(Int, getContext(), <b class="">Expr::SE_AllowSideEffects)</b>) -------> allow side effect</div><div class=""> return false; // Not foldable, not integer or not fully evaluatable.</div></div><div class=""><br class=""></div><div class="">We will not make the RHS constant folding happened.</div><div class=""><br class=""></div><div class=""><div class="">bool DataRecursiveIntBinOpEvaluator::</div><div class=""> VisitBinOpLHSOnly(EvalResult &LHSResult, const BinaryOperator *E,</div><div class=""> bool &SuppressRHSDiags) {</div><div class="">….</div><div class=""><br class=""></div><div class=""> if (E->isLogicalOp()) {</div><div class=""> bool LHSAsBool;</div><div class=""><span style="font-size: 10.5pt;" class=""> E->getLHS()->dumpColor();</span></div><div class=""> if (!LHSResult.Failed && HandleConversionToBool(LHSResult.Val, LHSAsBool)) {</div><div class=""> // We were able to evaluate the LHS, see if we can get away with not</div><div class=""> // evaluating the RHS: 0 && X -> 0, 1 || X -> 1</div><div class=""> if (LHSAsBool == (E->getOpcode() == BO_LOr)) {</div><div class=""> Success(LHSAsBool, E, LHSResult.Val);</div><div class=""> return false; // Ignore RHS</div><div class=""> }</div><div class=""> } else {</div><div class=""> LHSResult.Failed = true;</div><div class=""><br class=""></div><div class=""> // Since we weren't able to evaluate the left hand side, it</div><div class=""> // might have had side effects.</div><div class=""><b class=""> if (!Info.noteSideEffect()) { --------> HERE!!!</b></div><div class=""> return false;</div><div class=""> }</div></div><div class="">The detail source code analysis could be find in the comment 2 of <a href="https://bugs.llvm.org/show_bug.cgi?id=34229" class="" style="font-family: Arial; font-size: 10.5pt;">https://bugs.llvm.org/show_bug.cgi?id=34229</a></div><div class=""><br class=""></div><div class="">So, I think we should fix it. <br class=""></div></div></div></div></div></blockquote><div><br class=""></div>Are you arguing that we should generally ignore side-effects when constant-evaluating an if condition?</div><div><br class=""></div><div>John.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><div style="font-family: 'PingFang SC'; font-size: 10.5pt;" class=""><div class=""><div class="">
<div class="yomail-sig" style="margin:0;padding:0;"><div class="yomail-sig" style="padding: 0px 0px 0px 20px; margin: 0px 0px 0px -20px;"><span class=""> </span><div style="font-family: 'PingFang SC'; font-size: 10.5pt;" class=""><br class=""></div></div></div>
<br class="">
<div class="-eMc-quote" style="margin: 0px 0px 0px 0.8ex; display: block;">
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm" class="">
<div style="padding: 8px; font-size: 12px; background-color: rgb(239, 239, 239); background-position: initial initial; background-repeat: initial initial;" class="">
<div class="">
<b class="">发件人：</b> <a href="mailto:rjmccall@apple.com" target="_blank" class="">John McCall</a>
</div>
<div class="">
<b class="">发送时间：</b> 2017-08-23 01:30:45
</div>
<div class="">
<b class="">收件人：</b>
<a href="mailto:bluechristlove@163.com" target="_blank" class="">Frozen</a>
</div>
<div class="">
<b class="">抄送：</b>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" class="">cfe-dev@lists.llvm.org</a>
</div>
<div class="">
<b class="">主题：</b> Re: [cfe-dev] Evaluate constant condition of if statement has no effect
</div>
</div>
</div>
<blockquote style="margin:0;" class="">
<div dir="ltr" class="">
<div class="">
<div on_click="emc_toggle_blockquote_visibility(this);" class=""> </div>
<blockquote type="cite" class="">
<div class="">On Aug 21, 2017, at 12:28 AM, Frozen via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> wrote:</div>
<div class="">
<div style="line-height: 1.7; font-size: 14px; font-family: Arial;" class="">
<div class="">Simple Example:<br class="">
<br class="">
int main()</div>
<div class="">{</div>
<div class=""> int x;</div>
<div class=""> if (x || 1) {}</div>
<div class="">}</div>
<div class=""><br class=""></div>
<div class="">Clang can not evaluate this condition and will emit IR like this:</div>
<div class=""><br class=""></div>
<div class="">define signext i32 @main() #0 {</div>
<div class="">entry:</div>
<div class=""> %retval = alloca i32, align 4</div>
<div class=""> %x = alloca i32, align 4</div>
<div class=""> store i32 0, i32* %retval, align 4</div>
<div class=""> %0 = load i32, i32* %x, align 4</div>
<div class=""> %tobool = icmp ne i32 %0, 0</div>
<div class=""> br i1 %tobool, label %if.then, label %lor.lhs.false</div>
<div class=""><br class=""></div>
<div class="">lor.lhs.false: ; preds = %entry</div>
<div class=""> br i1 true, label %if.then, label %if.end</div>
<div class=""><br class=""></div>
<div class="">if.then: ; preds = %lor.lhs.false, %entry</div>
<div class=""> br label %if.end</div>
<div class=""><br class=""></div>
<div class="">if.end: ; preds = %if.then, %lor.lhs.false</div>
<div class=""> %1 = load i32, i32* %retval, align 4</div>
<div class=""> ret i32 %1</div>
<div class="">}</div>
<div class=""><br class=""></div>
<div class="">However, when we swap the position of LHS and RHS(i.e. if (1 || x), Clang can recognize it:</div>
<div class=""><br class=""></div>
<div class="">define signext i32 @main() #0 {</div>
<div class="">entry:</div>
<div class=""> %x = alloca i32, align 4</div>
<div class=""> ret i32 0</div>
<div class="">}<br class="">
<br class="">
I also find the root issue and propose one potential solution in comment 2 of this link: <a href="https://bugs.llvm.org/show_bug.cgi?id=34229" class="">https://bugs.llvm.org/show_bug.cgi?id=34229</a><br class="">
<br class="">
Any idea?<br class="">
</div>
</div>
</div>
</blockquote>
<div class=""><br class=""></div>
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?</div>
<div class=""><br class=""></div>
<div class="">John.</div>
</div>
</blockquote>
</div>
</div></div></div></div></div></blockquote></div><br class=""></blockquote></div><br><br><span title="neteasefooter"><p> </p></span>