[llvm-commits] [poolalloc] r159572 - /poolalloc/trunk/lib/AssistDS/DSNodeEquivs.cpp

[Matthew Wala]

On Mon, Jul 2, 2012 at 2:59 PM, John Criswell <criswell at illinois.edu> wrote:
> On 7/2/12 2:51 PM, Matthew Wala wrote:
>>
>> Author: wala1
>> Date: Mon Jul  2 14:51:52 2012
>> New Revision: 159572
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=159572&view=rev
>> Log:
>> Fix DSNodeEquivs to handle constant expressions; find some DSGraph an
>> expression belongs to by looking through its uses.
>>
>> Modified:
>>      poolalloc/trunk/lib/AssistDS/DSNodeEquivs.cpp
>>
>> Modified: poolalloc/trunk/lib/AssistDS/DSNodeEquivs.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/poolalloc/trunk/lib/AssistDS/DSNodeEquivs.cpp?rev=159572&r1=159571&r2=159572&view=diff
>>
>> ==============================================================================
>> --- poolalloc/trunk/lib/AssistDS/DSNodeEquivs.cpp (original)
>> +++ poolalloc/trunk/lib/AssistDS/DSNodeEquivs.cpp Mon Jul  2 14:51:52 2012
>> @@ -17,6 +17,9 @@
>>   #include "llvm/Constants.h"
>>   #include "llvm/Module.h"
>>   #include "llvm/Support/InstIterator.h"
>> +#include "llvm/ADT/SmallSet.h"
>> +
>> +#include <deque>
>>     namespace llvm {
>>   @@ -233,6 +236,46 @@
>>     } else if (isa<Argument>(V)) {
>>       const Function *Parent = cast<Argument>(V)->getParent();
>>       NHForV = &TDDS.getDSGraph(*Parent)->getNodeForValue(V);
>> +  } else {
>> +    //
>> +    // Iterate over the users to attempt to discover a DSNode that the
>> value
>> +    // maps to.
>> +    //
>> +    std::deque<const User *> WL;
>> +    SmallSet<const User *, 8> Visited;
>> +
>> +    WL.insert(WL.end(), V->use_begin(), V->use_end());
>> +    do {
>> +      const User *TheUser = WL.front();
>> +      WL.pop_front();
>> +
>> +      if (Visited.count(TheUser))
>> +        continue;
>> +      else
>> +        Visited.insert(TheUser);
>
>
> You should check and see if the insert method can tell you whether the
> insert succeeded because the element was not in the set or whether the
> insert failed because the element was already in the set.  This feature
> allows you to do a single operation on the set and should speed up the
> analysis.
>
>> +
>> +      //
>> +      // If the use is a global variable or instruction, then the value
>> should
>> +      // be in the corresponding DSGraph.
>> +      //
>> +      // TODO: Is it possible for a value to belong to some DSGraph and
>> never
>> +      // be relied upon by a GlobalValue or Instruction?
>> +      //
>> +      if (isa<Instruction>(TheUser)) {
>
>
> Why not use dyn_cast<Instruction> here?  That would avoid the
> cast<Instruction> in the line below.
>
>> +        const Function *Parent =
>> +          cast<Instruction>(TheUser)->getParent()->getParent();
>> +        NHForV = &TDDS.getDSGraph(*Parent)->getNodeForValue(V);
>> +        break;
>> +      } else if (isa<GlobalValue>(TheUser)) {
>> +        NHForV = &TDDS.getGlobalsGraph()->getNodeForValue(V);
>> +        break;
>> +      } else {
>> +        //
>> +        // If this use is of some other nature, look at the users of this
>> use.
>> +        //
>> +        WL.insert(WL.end(), TheUser->use_begin(), TheUser->use_end());
>> +      }
>> +    } while (!WL.empty());
>>     }
>
>
> You should have an assertion somewhere that ensures that you find a DSNode
> for the constant expression.
>

I'll add the above two optimizations you mentioned.

Isn't is possible that the constant doesn't have a DSNode? In that
case, it would be better to assert that a DSNodeHandle was found.

Matt

>
> -- John T.
>