[llvm-commits] [LLVMdev] SSI Patch

[Nick Lewycky]

Ping!

Nick Lewycky wrote:
> Andre Tavares wrote:
>> I made all changes. Hope it is good now.
> 
>       // Treat SSI_PHI
>       if (Variable *var = getVarFromPhi(PN)) {
>         var->addValueStack(PN);
>         defined.insert(var);
>       }
> 
>       // Treat SSI_SIG
>       else if (Variable *var = getVarFromPhi(PN)) {
>         substituteUse(I);
>         var->addValueStack(PN);
>         defined.insert(var);
>       }
> 
> Those are both testing getVarFromPhi. Should the second one be 
> getVarFromSigma instead?
> 
> And since we have to make one change, we may as well make others :)
> 
>   for (unsigned i = 0, e = PN->getNumIncomingValues(); i<e ; ++i) {
> 
> Whitespace.
> 
>     if (PHINode *PN_inc = dyn_cast<PHINode>(inc)) {
>       if (isSigPhi(PN_inc, I, Set))
>         continue;
>       return false;
>     }
>     return false;
>   }
> 
> Isn't that just:
> 
>     Value *PN_inc = dyn_cast<PHINode>(inc);
>     if (!PN_inc || !isSigPhi(PN_inc, I, Set))
>       return false;
>   }
> 
> given that the final } is closing the loop?
> 
> Finally, 'isSigPhi' is an ambiguous name. Perhaps it should be named 
> 'isSigma'?
> 
> Nick