[PATCH] Use discriminator information in sample profiles.

[Diego Novillo]

On Mon, Mar 10, 2014 at 12:54 PM, Chandler Carruth <chandlerc at gmail.com> wrote:

> ================
> Comment at: lib/Transforms/Scalar/SampleProfile.cpp:79-80
> @@ +78,4 @@
> +      : LineOffset(L), Discriminator(D) {}
> +  uint32_t LineOffset;
> +  uint32_t Discriminator;
> +};
> ----------------
> Are these wrapping? saturating? error if we reach UINT32_MAX (or whatever it is)? I'm not really worried either way, just wondering if it would be good to document it and enforce it with some asserts.

I've made the offset an int so we can check that it doesn't wrap
around.  Both should always be fairly small positive integers. I've
also changed all the other uses of uint32_t in the file. In
retrospect, it seems pointless not to just use unsigned ints here.


> ================
> Comment at: lib/Transforms/Scalar/SampleProfile.cpp:86
> @@ +85,3 @@
> +template<> struct DenseMapInfo<InstructionLocation> {
> +  typedef DenseMapInfo<uint32_t> Info;
> +  static inline InstructionLocation getEmptyKey() {
> ----------------
> Maybe HalfInfo or MemberInfo? Looking for some name that better identifies what it is...

Done.

> Related to my comment above about valid range -- I think this indicates the answer my question. If you can't have these uint32_t values, then it can't be wrapping, and if it is saturating you would need to write some code to saturate outside of these.
>
> We should at least document and place an assert around this?

Done. The line offsets are validated as we read them from the input
profile file.  I've added an assert to catch the impossible case.


Thanks. Diego.