<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="">Hi Renato<br class=""><div><blockquote type="cite" class=""><div class="">On Mar 12, 2015, at 4:07 AM, Renato Golin <<a href="mailto:renato.golin@linaro.org" class="">renato.golin@linaro.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">On 11 March 2015 at 23:00, Pete Cooper <<a href="mailto:peter_cooper@apple.com" class="">peter_cooper@apple.com</a>> wrote:<br class=""><blockquote type="cite" class="">This patch schedules another pass of Thumb2SizeReduction before the PostRA<br class="">scheduler.  This pass is only for functions with minsize set on them.  It<br class="">results in the mov being converted to Thumb2 before the PostRA scheduler and<br class="">so the code now becomes<br class=""></blockquote><br class="">Hi Pete,<br class=""><br class="">+  if (getOptLevel() != CodeGenOpt::None && !getARMSubtarget().isThumb1Only())<br class=""></div></blockquote>You’re right.  I’ve changed it to effectively be (getOptLevel() != CodeGenOpt::None && T2).  Then we have to run the pass over the functions and identify those functions which are -Oz.  Looks like -Oz is independent of -O0, but we probably still don’t want to even shrink the code if -O0 is set.<br class=""><blockquote type="cite" class=""><div class=""><br class="">This doesn't guarantee (Oz && T2). Wouldn't you have to be specific at<br class="">least regarding CodeGenOpt?<br class=""><br class="">Also, the same pass has already been added in some cases on the code<br class="">above, you don't want to add two identical passes, even if they have<br class="">different parameters. Maybe create a boolean OnlyMinSize from opt+t2<br class="">flags, and pass it to the already existing addPass()?<br class=""></div></blockquote>Agreed.  I’ve changed that snippet to be:</div><div><br class=""></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(49, 89, 93);" class=""><div style="margin: 0px;" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">    </span><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">bool</span><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> RestrictIT = </span>getARMSubtarget<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">().</span>restrictIT<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">();</span></div><div style="margin: 0px;" class="">    <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">if</span> (RestrictIT || getARMSubtarget().isThumb2()) {</div><div style="margin: 0px; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">      </span>// In v8, IfConversion depends on Thumb instruction widths.</div><div style="margin: 0px; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">      </span>// We also want to aggressively form Thumb2 instructions on functions</div><div style="margin: 0px; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">      </span>// with minsize (-Oz).</div><div style="margin: 0px;" class="">      <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">bool</span> OnlyMinSize = !RestrictIT;</div><div style="margin: 0px;" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">      </span>addPass<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">(</span>createThumb2SizeReductionPass<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">(OnlyMinSize));</span></div><div style="margin: 0px;" class="">    }</div></div></div><div><br class=""></div><div>I know I could have only one bool here, not 2, but I felt like this was easier to understand.  If you prefer I can make it a little shorter.</div><div><blockquote type="cite" class=""><div class=""><br class="">The tests are good and the rest looks good to me. Thanks!<br class=""></div></blockquote>Thanks!  I appreciate the review.</div><div><br class=""></div><div>Pete</div><div><br class=""></div><div></div></body></html>