[llvm-commits] [llvm] r80715 - in /llvm/trunk: include/llvm/Analysis/ProfileInfoLoader.h lib/Analysis/ProfileInfoLoader.cpp lib/Analysis/ProfileInfoLoaderPass.cpp

[Daniel Dunbar]

Hi Andreas,

One minor comment,

On Tue, Sep 1, 2009 at 12:08 PM, Andreas Neustifter<astifter at gmx.at> wrote:
> -  // Make sure we have enough space...
> +  // Make sure we have enough space... The space is initialised to -1 to
> +  // facitiltate the loading of missing values for OptimalEdgeProfiling.
>   if (Data.size() < NumEntries)
> -    Data.resize(NumEntries);
> +    Data.resize(NumEntries, -1);

FYI I changed this to ~0U to fix a build warning.

>   // Accumulate the data we just read into the data.
>   if (!ShouldByteSwap) {
> -    for (unsigned i = 0; i != NumEntries; ++i)
> -      Data[i] += TempSpace[i];
> +    for (unsigned i = 0; i != NumEntries; ++i) {
> +      unsigned data = TempSpace[i];
> +      if (data != (unsigned)-1) {       // only load data if its not MissingVal
> +        if (Data[i] == (unsigned)-1) {
> +          Data[i] = data;               // if data is still initialised
> +        } else {
> +          Data[i] += data;
> +        }
> +      }
> +    }
>   } else {
> -    for (unsigned i = 0; i != NumEntries; ++i)
> -      Data[i] += ByteSwap(TempSpace[i], true);
> +    for (unsigned i = 0; i != NumEntries; ++i) {
> +      unsigned data = ByteSwap(TempSpace[i], true);
> +      if (data != (unsigned)-1) {       // only load data if its not MissingVal
> +        if (Data[i] == (unsigned)-1) {
> +          Data[i] = data;
> +        } else {
> +          Data[i] += data;
> +        }
> +      }
> +    }

This would be simpler if there was a:
--
static const unsigned AddCounts(unsigned A, unsigned B) {
  // If either value is undefined, use the other.
  if (A == ~0U) return B;
  if (B == ~0U) return A;
  return A + B;
}
--
helper earlier, then it becomes obvious what the loops are doing:
--
    for (unsigned i = 0; i != NumEntries; ++i)
      Data[i] = AddCounts(Data[i], TempSpace[i]);
--

Do these values need to become uint64_t at some point?

 - Daniel