<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial">Firstly, I briefly clarify the logic of Info.noteSideEffect. Info.noteSideEffect will set <font face="arial"><span style="white-space: pre-wrap;">EvalStatus.HasSideEffects = true; and make </span></font><span style="font-family: arial; white-space: pre-wrap;">hasUnacceptableSideEffect function always return true so that our </span><style type="text/css">
p, li { white-space: pre-wrap; }
</style>
<div style="margin-top: 0px; margin-bottom: 0px;">EvaluateAsInt alway return false when LHS can not evaulate.</div><div style="margin-top: 0px; margin-bottom: 0px;"><br></div><div><font face="arial"><span style="white-space: pre-wrap;"><div style=""> if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isInt() ||</div><div style=""> hasUnacceptableSideEffect(ExprResult, AllowSideEffects))</div><div style=""> return false;</div><div style=""><br></div></span></font></div><div><font face="arial"><span style="white-space: pre-wrap;">static bool hasUnacceptableSideEffect(Expr::EvalStatus &Result,</span></font></div><div><font face="arial"><span style="white-space: pre-wrap;"> Expr::SideEffectsKind SEK) {</span></font></div><div><font face="arial"><span style="white-space: pre-wrap;"> return (SEK < Expr::SE_AllowSideEffects && Result.HasSideEffects) ||</span></font></div><div><font face="arial"><span style="white-space: pre-wrap;"> (SEK < Expr::SE_AllowUndefinedBehavior && Result.HasUndefinedBehavior);</span></font></div><div><font face="arial"><span style="white-space: pre-wrap;">}</span></font></div><div><br>Then I want to express one significant point of it, that is debug. Let us compare it with GCC in the following example.<br><br><div>int main()</div><div>{</div><div> int x;</div><div> if (1 || x) {</div><div> printf("LHS\n");</div><div> }</div><div><br></div><div> if (x || 1) {</div><div> printf("RHS\n");</div><div> }</div><div>}<br><br>When we use GCC to debug it, we will get the following result:<br><br><div>(gdb) b main</div><div>Breakpoint 1 at 0x10000620: file a.c, line 5.</div><div>(gdb) r</div><div>Starting program: a.out</div><div><br></div><div>Breakpoint 1, main () at a.c:5</div><div>5<span style="white-space:pre"> </span> printf("LHS\n");</div><div>Missing separate debuginfos, use: debuginfo-install glibc-2.17-78.ael7b.ppc64le</div><div>(gdb) step</div><div>LHS</div><div>9<span style="white-space:pre"> </span> printf("RHS\n");</div><div>(gdb) step</div><div>RHS</div><div>11<span style="white-space:pre"> </span>}</div></div></div><br><div>And if we use clang, we will get:<br><div>(gdb) b main</div><div>Breakpoint 1 at 0x10000614: file a.c, line 5.</div><div>(gdb) r</div><div>Starting program: a.out</div><div><br></div><div>Breakpoint 1, main () at a.c:5</div><div>5<span style="white-space:pre"> </span> printf("LHS\n");</div><div>Missing separate debuginfos, use: debuginfo-install glibc-2.17-78.ael7b.ppc64le</div><div>(gdb) step</div><div>LHS</div><div>8<span style="white-space:pre"> </span> if (x || 1) {</div><div>(gdb) step</div><div>9<span style="white-space:pre"> </span> printf("RHS\n");</div><div>(gdb) step</div><div>RHS</div><div>11<span style="white-space:pre"> </span>}</div></div><div><br>We can see that we will stop at line 8 and can not stop at line 9 directly like GCC or LHS is constant. This is somehow not unified behaviour and make uses feel strange.</div><div style="position:relative;zoom:1"></div><div id="divNeteaseMailCard"></div><br>在 2017-08-24 13:59:06，"John McCall" <rjmccall@apple.com> 写道：<br> <blockquote id="isReplyContent" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><div><blockquote type="cite" class=""><div class="">On Aug 24, 2017, at 1:26 AM, Frozen <<a href="mailto:bluechristlove@163.com" class="">bluechristlove@163.com</a>> wrote:</div><div class=""><div style="line-height: 1.7; font-size: 14px; font-family: Arial;" class=""><div class="">Not. Original implementation used the default parameter value, i.e. No Side Effects<br class=""><div class=""><br class=""></div><div class=""> /// EvaluateAsInt - Return true if this is a constant which we can fold and</div><div class=""> /// convert to an integer, using any crazy technique that we want to.</div><div class=""> bool EvaluateAsInt(llvm::APSInt &Result, const ASTContext &Ctx,</div><div class=""> <b class="">SideEffectsKind AllowSideEffects = SE_NoSideEffects</b>) const;<br class=""><br class="">But, now, we should <b class="">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 class=""><div class=""><br class=""></div><div class="">bool DataRecursiveIntBinOpEvaluator::</div><div class=""> VisitBinOpLHSOnly(EvalResult &LHSResult, const BinaryOperator *E,</div><div class=""> bool &SuppressRHSDiags) {</div><div class=""> if (E->isLogicalOp()) {</div><div class=""> bool LHSAsBool;</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=""><b class=""> // Since we weren't able to evaluate the left hand side, it</b></div><div class=""><b class=""> // might have had side effects.</b></div><div class=""><b class=""> if (!Info.noteSideEffect())</b></div><div class=""><b class=""> return false;</b></div><div class=""><br class=""></div><div class=""> // We can't evaluate the LHS; however, sometimes the result</div><div class=""> // is determined by the RHS: X && 0 -> 0, X || 1 -> 1.</div><div class=""> // Don't ignore RHS and suppress diagnostics from this arm.</div><div class=""> SuppressRHSDiags = true;</div><div class=""> }</div><div class=""><br class=""></div><div class=""> return true;</div><div class=""> }</div></div><div class=""><br class=""></div><div class="">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 class="">Info.noteSideEffect() </b>to compute the result. </div></div></div></blockquote><div><br class=""></div>Info.noteSideEffect() is not given the expression to evaluate, nor does it call back to IR-generation in a way that would allow us to emit the side effect. A more promising approach would be to modify IRGen's logic for emitting branches on conditions, which is already a special-case expression emitter due to the desire to directly branch on && and || instead of branching on a phi. That emitter has special-case logic for constant branches, but from the generated code it looks like we're not applying that in all positions, just at the top level. Recognizing when we're about to branch on a constant i1 would be sufficient to catch things like this and allow IRGen to reap the (small) benefits of emitting less silly code. I don't think there's much point in trying to recognize (X || true) especially, but we could do that, too, if you have a good argument.</div><div><br class=""></div><div>John.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="line-height: 1.7; font-size: 14px; font-family: Arial;" class=""><br class=""><div style="position:relative;zoom:1" class=""></div><div class=""></div><br class="">在 2017-08-24 02:02:23，"John McCall" <<a href="mailto:rjmccall@apple.com" class="">rjmccall@apple.com</a>> 写道：<br class=""> <blockquote id="isReplyContent" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid" class=""><br class=""><div class=""><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 class=""><br class=""></div>Are you arguing that we should generally ignore side-effects when constant-evaluating an if condition?</div><div class=""><br class=""></div><div class="">John.</div><div class=""><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 class=""><br class=""><span title="neteasefooter" class=""><div class=""> <br class="webkit-block-placeholder"></div></span></div></blockquote></div><br class=""></blockquote></div><br><br><span title="neteasefooter"><p> </p></span>