[llvm-commits] [llvm] r162799 - in /llvm/trunk: include/llvm/Analysis/Passes.h include/llvm/Analysis/ProfileDataLoader.h include/llvm/InitializePasses.h include/llvm/LinkAllPasses.h lib/Analysis/Analysis.cpp lib/Analysis/CMakeLists.txt lib/Analysis/ProfileDataLoader.cpp lib/Analysis/ProfileDataLoaderPass.cpp lib/Analysis/ProfileInfo.cpp test/Analysis/Profiling/load-branch-weights-ifs.ll test/Analysis/Profiling/load-branch-weights-loops.ll test/Analysis/Profiling/load-branch-weights-switches.ll

[Alastair Murray]

Hi Bill, All,

Thank you for committing the previous patch.

Attached is a patch to make use of MemoryBuffer instead of fopen/fread. 
  This patch should be applied _after_ pulling down Benjamin Kramer's 
changes (he already committed those in rev 162799).

The one line change in ProfileDataLoaderPass.cpp within the patch is 
technically unrelated, but is obvious.  It relates to producing correct 
error messages.

For the MemoryBuffer changes, there is a ReadBuffer<T> function which is 
used to read arbitary types from the MemoryBuffer.  This contains 
pointer casts, which may be unacceptable.  I've contained this aspect 
within the one function, but please let me know your thoughts as I'm 
certainly open to a cleaner solution.

I do not have commit access, so if the patch is okay then please commit it.

Regards,
Alastair.

On 31/08/12 01:21, Bill Wendling wrote:
> On Aug 30, 2012, at 4:23 PM, Alastair Murray <alastairmurray42 at gmail.com> wrote:
>
>> Hi Bill,
>>
>> On 30/08/12 21:24, Bill Wendling wrote:
>>> The patch looks fine. Thank you for doing it! Let me know if you don't have commit access and I can apply it for you.
>>
>> I do not have commit access, so if you could please apply it it would be appreciated!
>>
> Okay. Done in r162979. Thanks!
>
>>> If the namespace issue is messing things up, then you can leave it. It was a minor issue. I'm still not 100% convinced about the weight code. I doubt that you're gaining much by expanding to uint64_t and then back to unsigned. If anything, you should probably just use uint64_t directly in the whole function instead of casting (which, gross):
>>>
>>> static uint64_t AddCounts(uint64_t A, uint64_t B) {
>>>    // ...
>>> }
>>>
>>> Would this address your concerns?
>>
>> Yes, I could do this.  Reading between the lines, however, I could just not saturate on overflow but leave it unhandled?  Is that preferable?
>>
> If you could give an example of what you're trying to do, that might be better. We may not be talking about the same thing. What does the math look like that you're trying to handle by going to uint64_t?
>
> -bw
>
>> I'll try and get the MemoryBuffer patch written tomorrow.
>>
>> Regards,
>> Alastair.
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: weights_6_membuffer.patch
Type: text/x-patch
Size: 7287 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120901/d16b2366/attachment.bin>