<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Committed in r177331.<div><br><div apple-content-edited="true">
<div style="color: rgb(0, 0, 0); font-family: Helvetica; font-size: medium; 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-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">-Quentin</div>
</div>
<br><div><div>On Mar 18, 2013, at 3:01 PM, Bill Wendling <<a href="mailto:wendling@apple.com">wendling@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">LGTM. Thanks, Quentin! :)<br><br>-bw<br><br>On Mar 18, 2013, at 2:32 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br><blockquote type="cite">Thanks Bill.<br><br>Here is the updated patch.<br><br>-Quentin<br><br><GlobalMergeOnConst.patch><br>On Mar 18, 2013, at 1:50 PM, Bill Wendling <<a href="mailto:wendling@apple.com">wendling@apple.com</a>> wrote:<br><br><blockquote type="cite">On Mar 18, 2013, at 1:28 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br><blockquote type="cite">Thanks for the feedbacks Duncan.<br><br>Here is the updated version of the patch as well as the updated compile time numbers.<br>The updated patch just checks for the “used” attribute and the uses in landing pad instructions. More particularly, it looks for all landing pad instructions in the module and marks internally the involved global variables as “must keep”.<br><br>For the compile time numbers, on average performing the new checks plus merging the constants (SPEC-compile_time_with_const.txt) does not have any noticeable impact on the compile time now.<br><br></blockquote>Looking good! One comment:<br><br>+void GlobalMerge::setMustKeepGlobalVariables(Module &M) {<br>+ // If we already processed a Module, UsedGlobalVariables may have been<br>+ // populated. Reset the information for this module.<br>+ MustKeepGlobalVariables.clear();<br>+ collectUsedGlobalVariables(M);<br>+<br>+ for (Module::iterator IFn = M.begin(), IEndFn = M.end(); IFn != IEndFn;<br>+ ++IFn) {<br>+ for (Function::iterator IBB = IFn->begin(), IEndBB = IFn->end();<br>+ IBB != IEndBB; ++IBB) {<br>+ const LandingPadInst *LPInst = IBB->getLandingPadInst();<br>+ if (!LPInst)<br>+ continue;<br><br>As a micro-optimization, you might want to rewrite this as:<br><br><span class="Apple-tab-span" style="white-space: pre;"> </span>const InvokeInst *II = dyn_cast<InvokeInst>(IBB->getTerminator());<br><span class="Apple-tab-span" style="white-space: pre;"> </span>if (!II) continue;<br><br><span class="Apple-tab-span" style="white-space: pre;"> </span>const LandingPadInst *LPInst = II->getUnwindDest()->getLandingPadInst();<br><br>This way it's not looping through the beginning of a basic blocks unless it has to.<br><br>+ // Look for globals in the operands of the landing pad instruction<br>+ for (unsigned OpIdx = 0, EndOpIdx = LPInst->getNumOperands();<br>+ OpIdx != EndOpIdx; ++OpIdx) {<br><br>Instead of looping through all operands, you might want to concentrate on just the clauses:<br><br><span class="Apple-tab-span" style="white-space: pre;"> </span>for (unsigned Idx = 0, NumClauses = LPInst->getNumClauses(); Idx != NumClauses; ++Idx)<br><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span> if (const GlobalVariable *GV =<br><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>dyn_cast<GlobalVariable>(LPInst->getClause(Idx)->stripPointerCasts()))<br><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span> MustKeepGlobalVariables.insert(GV);<br><br>The other uses of globals inside of the LandingPadInst shouldn't be candidates for merging already (or if they are, it's not a problem).<br><br>+ const GlobalVariable *GV =<br>+ dyn_cast<GlobalVariable>(LPInst->getOperand(OpIdx)-><br>+ stripPointerCasts());<br>+ if (GV)<br>+ MustKeepGlobalVariables.insert(GV);<br>+ }<br>+ }<br>+ }<br>+}<br><br>-bw</blockquote></blockquote></div></blockquote></div><br></div></body></html>