[PATCH] blockfreq: Rewrite block frequency analysis
atrick at apple.com
Wed Mar 19 14:23:12 PDT 2014
On Mar 19, 2014, at 1:38 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> On Mar 19, 2014, at 12:29 AM, Andrew Trick <atrick at apple.com> wrote:
>> On Mar 7, 2014, at 3:44 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>>> This patch series rewrites the shared implementation of BlockFrequencyInfo
>>> and MachineBlockFrequencyInfo entirely.
>>> - Patches 0001 and 0002 are simple cleanups.
>>> - Patch 0003 is a class rename (and file move). Let me know if I should
>>> just merge it with 0006.
>>> - Patches 0004 and 0005 introduce some supporting classes.
>>> - Patch 0006 rewrites BlockFrequencyInfoImpl entirely.
>>> - Patch 0007 is *not* included (it’s headed separately to llvmdev).
>> These patches all look pretty great to me.
> Thanks everyone for the reviews so far. There seems to be some consensus that
> the direction of PositiveFloat is a good one (as long as it shares effort with
> APFloat to minimize maintenance burden), and I’ve had a couple of LGTMs on the
> overall algorithm.
> What’s the path forward? I propose the following:
> 0. clang-format :).
> 1. Commit patches 0001/0002.
> 2. Put the math from PositiveFloat into reusable helper class (or namespace?),
> and factor out the common components from APFloat. I'll work on this
> incrementally in the tree, and port my existing testcases from
> PositiveFloat as necessary.
> 3. Add the math from BlockMass that needs testing to the same helper class
> (namespace?). This will allow BlockMass to be a local implementation
> detail of BlockFrequencyInfoImpl, sidestepping concerns about lib/Support
> 4. Commit PositiveFloat.cpp, which will at this point be mostly API boilerplate
> (operators, etc.).
> 5. Commit BlockFrequencyInfoImpl in the new location, leaving the old
> BlockFrequencyImpl in place and in use. Incrementally apply review feedback
> starting with what I’ve already received in this thread and the RFC.
Sounds good to me.
> 6. “Turn on” the new pass implementation, and then delete the old one.
Yes, after checking compile time and deciding how we should verify the implementation now and after future changes.
> 7. Commit the bias metric (current patch 0007), so that people can experiment
> with using that instead of block frequency.
Yes, after resolving questions on the RFC thread.
> Does this sound good to everyone?
> Chandler, do you still want something up on Phab at some point? If so, does it
> makes sense for me to post the current, massive, combined patch of
> 0003+0004+0005+0006(+0007?) now, so you can look at the algorithm as a whole?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits