[llvm-commits] [llvm] r56341 - in /llvm/trunk: include/llvm/ include/llvm/Transforms/ lib/Transforms/IPO/ test/Analysis/GlobalsModRef/ test/Transforms/AddReadAttrs/

[Chris Lattner]

On Sep 19, 2008, at 1:17 AM, Duncan Sands wrote:

> Author: baldrick
> Date: Fri Sep 19 03:17:05 2008
> New Revision: 56341
>
> URL: http://llvm.org/viewvc/llvm-project?rev=56341&view=rev
> Log:
> Add a new pass AddReadAttrs which works out which functions
> can get the readnone/readonly attributes, and gives them it.
> The plan is to remove markmodref (which did the same thing
> by querying GlobalsModRef) and delete the analogous
> functionality from GlobalsModRef.

Ok.  Here are some thoughts:

   CallGraph &CG = getAnalysis<CallGraph>();

   // Check if any of the functions in the SCC read or write memory.
   // If they write memory then just give up.

...

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:

if (SCC.size() == 1 && SCC[0]->getFunction() &&
     SCC[0]->getFunction()->doesNotAccessMemory())
   return false;  // already readnone

Alternatively, maybe this should be handled by the 'already perfect'  
case you already have.


     // Definitions with weak linkage may be overridden at linktime with
     // something that writes memory, so treat them like declarations.
     if (F->isDeclaration() || F->hasWeakLinkage()) {

This should also handle linkonce linkage.

       // Ignore calls to functions in the same SCC.
       if (CS.getInstruction() &&
           std::find(SCC.begin(), SCC.end(),  
CG[CS.getCalledFunction()]) !=
           SCC.end())
         continue;

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.


       if (II->mayWriteToMemory())
         return false;

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.

Otherwise, the pass looks very nice Duncan!

-Chris


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080921/2b52bfed/attachment.html>