<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Oct 10, 2011, at 1:36 PM, Evan Cheng wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Oct 10, 2011, at 1:32 PM, Devang Patel wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Oct 10, 2011, at 1:20 PM, Evan Cheng wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; ">On Oct 10, 2011, at 12:09 PM, Devang Patel wrote:<br><br><blockquote type="cite"><br></blockquote><blockquote type="cite">/// Hoist - When an instruction is found to use only loop invariant operands<br></blockquote><blockquote type="cite">/// that are safe to hoist, this instruction is called to do the dirty work.<br></blockquote><blockquote type="cite">///<br></blockquote><blockquote type="cite">@@ -1139,6 +1167,8 @@<br></blockquote><blockquote type="cite"> MI = ExtractHoistableLoad(MI);<br></blockquote><blockquote type="cite"> if (!MI) return false;<br></blockquote><blockquote type="cite"> }<br></blockquote><blockquote type="cite">+ if (!IsGuaranteedToExecute(MI))<br></blockquote><blockquote type="cite">+ return false;<br></blockquote><br>Why is the check here? Shouldn't the check be in IsLICMCandidate()?</span></blockquote></div><br><div>I put, potentially compile time expensive, dominator check in the end.</div></div></blockquote><div><br></div>You should only do the check when the instruction being considered is not side-effect free. That should eliminate some of the concerns about compile time. Putting the check not only is the wrong (from the design aspect), it also means LICM is doing unnecessary work in IsLoopInvariantInst, IsProfitableToHoist, and ExtractHoistableLoad.</div></div></blockquote><div><br></div>Also note, the fix as it is, has performance impact. We do want to speculate when the instructions are cheap and register pressure is low.</div><div><br></div><div>Evan</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>Evan</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>-</div><div>Devang</div></div></blockquote></div><br></div></blockquote></div><br></body></html>