[llvm-commits] [PATCH] Clear unknown mem ops when merging stack slots (pr14090)

Matthew Curtis mcurtis at codeaurora.org
Wed Oct 17 12:25:12 PDT 2012


On 10/17/2012 1:45 PM, Nadav Rotem wrote:
> Looking good!  Please commit.  I have a small comment below.
>
> +        const Value *UO = GetUnderlyingObject(V);
> +
> +        // Update value if it is in our map. Otherwise we may need to clear it
> +        // if we don't know for sure that it doesn't alias a merged alloca.
> +        DenseMap<const Value*, const Value*>::iterator NewV = Allocas.find(UO);
> +        if (NewV != Allocas.end())
>
> Why do you use 'find' and not count ?
Hmmm, just subjective really. I can change it back to count if you'd like.

>   
>
> +          V = NewV->second;
> +        else if (!UO || !isa<AllocaInst>(UO))
> +          V = 0;
>
> Are you checking if UO is zero after searching the map because you assume that it is less likely to happen ?
It made the code more readable, although this was somewhat related to 
the use of the iterator. If I go back to using count, the code would 
look like this:

         if (!UO)
           V = 0;
         else if (Allocas.count(UO))
           V = Allocas[UO];
         else if (!isa<AllocaInst>(UO))
           V = 0;

That actually seems better to me.

>
> +
> +        MMO->setValue(V);
> +        FixedMemOp += (V != 0);
>
> Thanks,
> Nadav
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation




More information about the llvm-commits mailing list