[llvm-commits] [llvm] r80907 - /llvm/trunk/lib/Analysis/ProfileInfoLoaderPass.cpp

[Andreas Neustifter]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi!

Andreas Neustifter wrote:
> Hi,
> 
> Daniel Dunbar wrote:
>> On Thu, Sep 3, 2009 at 12:17 PM, Andreas Neustifter<astifter-llvm at gmx.at> wrote:
>>>> On Thu, Sep 3, 2009 at 1:32 AM, Andreas Neustifter<astifter-llvm at gmx.at> wrote:
>>>>>> On Thu, Sep 3, 2009 at 12:53 AM, Andreas Neustifter<astifter-llvm at gmx.at> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> thank you very much for investigating this!
>>>>>>>
>>>>>>> Daniel Dunbar wrote:
>>>>>>>> -  if (weight != (unsigned)MissingValue) {
>>>>>>>> +  if (weight != ~0U) {
>>>>>>> Since allmost all my other development takes place on platforms that do
>>>>>>> not even support floating point my float-typecasting knowledge is a bit
>>>>>>> rusty.
>>>>>>>
>>>>>>> If you have time: can you explain why this is necessary and why the cast
>>>>>>> from double to unsigned does not work? (At least not on all platforms...)
>>>>>> I didn't know this off the top of my head either, but the relevant
>>>>>> language for C99 (I presume C++ is similar is):
>>>>>> --
>>>>>> 6.3.1.4p1: When a finite value of real floating type is converted to
>>>>>> an integer type other than _Bool, the fractional part is discarded
>>>>>> (i.e., the value is truncated toward zero). If the value of the
>>>>>> integral part cannot be represented by the integer type, the behavior
>>>>>> is undefined.
>>>>>> --
>>>>> Thanks, so maybe we should convert all the counters to (signed) int64_t
>>>>> to clean up this problem and get larger counters...
>>>> Yes, but this particular problem is because some parts of the
>>>> interface are using 'unsigned' to represent counts, and other parts
>>>> are using double. In this case the comparison was really against the
>>>> sentinel value from the file format, not the sentinel value from the
>>>> ProfilingInfo interface.
>>> Ja, I think I got that. What I meant its that if the file format
>>> sentinel is (int64_t)-1 and the ProfileInfo::MissingValue is double then
>>> this cast would work because (double)-1 can be represented in int64_t.
>>>
>>> No?
>> It would, but I think it would still conceptually be wrong, since
>> conceptually the two values have different "types". The type of one is
>> (conceptually) ProfileInfoEdgeWeightType, and the type of the other is
>> ProfileDataFileEdgeWeightType. 
> 
> Ah, I was thinking purely technical. :-)
> 
>> It would be nice if the code was
>> actually very clear about when it was transitioning from one type to
>> the other.
> 
> That could be arranged, I will look to it.

Okay, attached is a patch that (tries) to clean up that sentinel mess.

I'm not sure tough if its the proper way to proceed, please have a look.

Andi

- --
==========================================================================
This email is signed, for more information see
http://web.student.tuwien.ac.at/~e0325716/gpg.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqhFCIACgkQPiYq0rq7s/Am5gCeL+VgWzeLLDCcuPmdrFPtETn0
UaUAnjPQBXDUPbzzrnQsjlMWzS0cutNl
=SIS9
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-r81001.sentinel.cleanup.patch
Type: text/x-patch
Size: 7205 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090904/d38c8c1c/attachment.bin>