<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Thank you! In r240072.<br class=""><div><blockquote type="cite" class=""><div class="">On Jun 18, 2015, at 3:30 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">LGTM.<div class=""><br class=""></div><div class="">Thanks Yi!</div><div class=""><br class=""></div><div class="">Q.<br class=""><div class=""><blockquote type="cite" class=""><div class="">On Jun 18, 2015, at 3:26 PM, yi_jiang <<a href="mailto:yjiang@apple.com" class="">yjiang@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Quentin, <div class=""><br class=""></div><div class="">Thank you for your comments! I made some adjustments on the new patch. For the message in the == case, I tried a few settings while found that printing nothing looks like the best since the new PHI node will be printed right way. Here is the example:</div><div class=""><br class=""></div><div class=""><div class="">If-converting %vreg1<def> = PHI %vreg0, <BB#0>, %vreg3, <BB#4>, %vreg3, <BB#3>; GPR32common:%vreg1 GPR32all:%vreg0,%vreg3,%vreg3</div><div class=""> --> %vreg1<def> = PHI %vreg0, <BB#0>, %vreg3, <BB#2>; GPR32common:%vreg1 GPR32all:%vreg0,%vreg3</div><div class="">If-converting %vreg2<def> = PHI %vreg8, <BB#0>, %vreg5, <BB#4>, %vreg4, <BB#3>; GPR32common:%vreg2,%vreg8 GPR32all:%vreg5,%vreg4</div><div class=""> --> %vreg18<def> = CSINCWr %vreg5, %vreg2, 10, %NZCV<imp-use>; GPR32common:%vreg18,%vreg2 GPR32:%vreg5</div><div class=""> --> %vreg2<def> = PHI %vreg8, <BB#0>, %vreg18, <BB#2>; GPR32common:%vreg2,%vreg8,%vreg18</div></div><div class=""><div class=""></div></div></div><span id="cid:BC5D79A3-EE19-4588-BB40-78115C5A81A5" class=""><ifcvt2.patch></span><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""></div><div class=""> --> %vreg2<def> = PHI %vreg8, <BB#0>, %vreg18, <BB#2>; GPR32common:%vreg2,%vreg8,%vreg18</div></div><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Jun 18, 2015, at 1:29 PM, Quentin Colombet <qcolombet@<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__apple.com_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=beOkNeBNvqP3ST0ainEgQu8KuZKKJ2pc-KDWPJKB3tc&s=YC2FUNlxIRuMzyjTkyYOITLJzgKQBRJJXefEdUoTbvw&e=" class="">apple.com</a>> wrote:</div></blockquote><blockquote type="cite" class=""><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Hi Yi,</span><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">The patch looks good to me with a few nitpicks. Nice catch!</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">A couple of nitpicks:</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">1.</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="">+ if (PI.TReg == PI.FReg) {</div><div class="">+ DstReg = PI.TReg;</div><div class="">+ } else {</div><div class="">+ DEBUG(dbgs() << "If-converting " << *PI.PHI);</div><div class=""><br class=""></div><div class="">Please move this DEBUG output before the if, and print a message in the == case.</div><div class=""><br class=""></div><div class="">2.</div><div class="">Although this is obvious what you are doing, I would like a comment saying that we do not need the select when both sources are equal.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">-Quentin</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div class="">On Jun 18, 2015, at 12:34 PM, yi_jiang <<a href="mailto:yjiang@apple.com" class="">yjiang@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class="">Hi, </div><div class=""><br class=""></div><blockquote class="" style="margin: 0px 0px 0px 40px; border: none; padding: 0px;"></blockquote>We found that early if-conversion pass may introduce some redundant select node. Please see the example below, I use llvm IR here just to make it a little easier to read although early if-conversion is dealing machine instructions. When we if-convert a diamond below, If the tail basic block(while.body) has extra predecessors(while.body.lr.ph), we need to insert select in the head basic block(while.cond) and rewrite the PHI in the tail basic block(while.body). However, if this select node is actually selecting same value, that is redundant. In our case, it is the %j.012 with incoming value of %inc from both %if.then and %if.else. And since early if conversation is pretty late in codegen so the latter passes won’t be able to remove this redundancy. This patch is to remove this redundancy. Bzip2 decompression will benefit from this patch. <div class=""><div class=""><br class=""></div><div class=""><div class="">while.cond: ; Head BB</div><div class=""> ...</div><div class=""> br i1 %cmp2, label %if.then, label %if.else</div><div class=""><br class=""></div><div class="">if.then: ;TBB</div><div class=""> %inc2 = add nsw i32 %i.011, 1</div><div class=""> br label %while.body</div><div class=""><br class=""></div><div class="">if.else: ;FBB</div><div class=""> %dec = add nsw i32 %i.011, -1</div><div class=""> br label %while.body</div><div class=""><br class=""></div></div><div class=""><div class="">while.body: ;Tail BB</div><div class=""> %j.012 = phi i32 [ %sub, %while.body.lr.ph ], [ %inc, %if.then ], [ %inc, %if.else ] ;we dont need a select of %inc and %inc from the TBB/FBB in Head BB</div><div class=""> %i.011 = phi i32 [ %a, %while.body.lr.ph ], [ %inc2, %if.then ], [ %dec, %if.else ] ;we DO need a select of %inc2 and %dec from the TBB/FBB in Head BB</div><div class=""> ...</div><div class=""> br i1 %cmp1, label %while.end, label %while.cond</div></div><div class=""><br class=""><blockquote class="" style="margin: 0px 0px 0px 40px; border: none; padding: 0px;"></blockquote><br class=""></div></div></div><span id="cid:1A8270A5-803B-4FC1-B190-AAF19E05510A" class=""><ifcvt.patch></span><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><div class=""><div class=""><br class=""></div><br class=""></div></div></div>_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div></div></div></blockquote></div><br class=""></div></div></div></blockquote></div><br class=""></div></div></div></blockquote></div><br class=""></body></html>