<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Sep 19, 2008, at 1:17 AM, Duncan Sands wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Author: baldrick<br>Date: Fri Sep 19 03:17:05 2008<br>New Revision: 56341<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=56341&view=rev">http://llvm.org/viewvc/llvm-project?rev=56341&view=rev</a><br>Log:<br>Add a new pass AddReadAttrs which works out which functions<br>can get the readnone/readonly attributes, and gives them it.<br>The plan is to remove markmodref (which did the same thing<br>by querying GlobalsModRef) and delete the analogous<br>functionality from GlobalsModRef.</div></blockquote><div><br></div></div>Ok.  Here are some thoughts:<div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">  CallGraph &CG = getAnalysis<CallGraph>();</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; min-height: 14px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000">  </span>// Check if any of the functions in the SCC read or write memory.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000">  </span>// If they write memory then just give up.</div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;">...</span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;">This pass can only do something if the SCC is not already readnone.  Does it make sense to do an initial check to see if the function is already readnone?  Since it doesn't have to be a perfect check, how about something like:</span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;">if (SCC.size() == 1 && SCC[0]->getFunction() && </span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;">    SCC[0]->getFunction()->doesNotAccessMemory())</span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;">  return false;  // already readnone</span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;">Alternatively, maybe this should be handled by the 'already perfect' case you already have.</span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000">    </span>// Definitions with weak linkage may be overridden at linktime with</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000">    </span>// something that writes memory, so treat them like declarations.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">    <span style="color: #aa0d91">if</span> (F->isDeclaration() || F->hasWeakLinkage()) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">This should also handle linkonce linkage.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000">      </span>// Ignore calls to functions in the same SCC.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">      <span style="color: #aa0d91">if</span> (CS.getInstruction() &&</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">          std::find(SCC.begin(), SCC.end(), CG[CS.getCalledFunction()]) !=</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">          SCC.end())</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">        <span style="color: #aa0d91">continue</span>;</div><div><br></div><div>This will be really slow for large SCCs.  I think that some apps (like 176.gcc) have scc's with hundreds of nodes in them.  Instead of using std::find, please dump the contents of the SCC into a SmallPtrSet at the start of the function, and query that instead.</div><div><br></div></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><br></div></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">      <span style="color: #aa0d91">if</span> (II->mayWriteToMemory())</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(170, 13, 145); "><span style="color: #000000">        </span>return<span style="color: #000000"> </span>false<span style="color: #000000">;</span></div><div><font class="Apple-style-span" color="#000000"><br></font></div><div><font class="Apple-style-span" color="#000000">Please add a comment: "If this may write to memory, then we can't set either readnone or readonly, just give up." or something like that.</font></div><div><font class="Apple-style-span" color="#000000"><br></font></div><div><font class="Apple-style-span" color="#000000">Otherwise, the pass looks very nice Duncan!</font></div><div><font class="Apple-style-span" color="#000000"><br></font></div><div><font class="Apple-style-span" color="#000000">-Chris</font></div></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" color="#007400" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div></div></body></html>