[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

Bob Wilson bob.wilson at apple.com
Mon Oct 29 10:28:02 PDT 2012


I've committed it in svn 166938.

On Oct 26, 2012, at 7:58 PM, Alastair Murray <alastairmurray42 at gmail.com> wrote:

> Hi Bill, all,
> 
> It's been a while since you mentioned this.  I just re-examined the code and the use-case for performing arithmetic in 64-bits likely rarely makes any difference (and saturating is the wrong thing to do anyway).
> 
> I've been thinking changing how profiling counter values are stored, but that won't be done any time soon.
> 
> The attached patch just removes the 64-bit and saturation code.
> 
> I do not have commit access, so if the patch is suitable please apply 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.
>> 
>> 
> 
> <weights_7_simplify_add.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list