[PATCH] blockfreq: Rewrite block frequency analysis

[Duncan P. N. Exon Smith]

On Apr 3, 2014, at 14:11, Andrew Trick <atrick at apple.com> wrote:

> 
> On Mar 26, 2014, at 5:43 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
>> 
>> On Mar 25, 2014, at 7:23 PM, Chandler Carruth <chandlerc at google.com> wrote:
>> 
>>> 
>>> Ok. I think having the terminology clarified will also help me understand the code.
>>> 
>>> 
>>> The only thing that I think really needs to be sorted out (other than the stuff you've clarified you're planning to do already) prior to commit is the positive float issue. I'm just a bit hesitant to put that into Support only to rip it back out. ;] But maybe that doesn't matter too much.
>> 
>> FYI, I committed the preparatory patches recently: r204740 and r204741.
>> 
>> Here’s a new plan for the remainder that minimizes churn in Support:
>> 
>> 1. Move BlockMass and PositiveFloat into private headers.  Strip their
>>   tests from the patch, but hang onto them out of tree.
>> 
>> 2. Commit everything as a new class that’s unused.
>> 
>> 3. Move non-trivial things I need from BlockMass and PositiveFloat into
>>   Support (sharing implementation details with APFloat, where possible). 
>>   I’ll commit tests for the functionality I need.
>> 
>> 4. Switch from soft-float to power-of-two-except-3 loop-scale.  (Delete
>>   PositiveFloat at this point.)
>> 
>> 5. Switch to LoopInfo.
>> 
>> 6. Respond to the rest of the review comments incrementally.
>> 
>> 7. Turn it on.
>> 
>> 8. Kill the old one.
>> 
>> Sound good?
> 
> It would be great to get a working design committed and begin making the improvements we discussed. However, I don’t think you need to keep the existing implementation alive as you replace pieces. There’s isn’t any need here to preserve old broken behavior. Ideally the first commit will be usable and easy to review even if it’s not efficient or missing features.
> 
> 1. Move to using existing LoopInfo to rule out that as a source of problems.
> 
> 2. Move BlockMass and PositiveFloat out of support (make them temporary helpers)
> 
> 3. Commit and enable.
> 
> 4. Factor Float/Mass utilities into Support
> 
> 5. Switch to pow2 loop scale
> 
> 6. Reimplement Positive float as a very simple BlockFrequency-specific wrapper that implements loop scale computation and frequency scaling in terms of integer operations (you seem to have come up with a way to do this that won’t lead to madness).
> 
> One more thing. Chandler requested definitions of the terminology, which would be good to do before your first commit.
> 
> -Andy

Andy: thanks, this plan is better.

I just pushed a bunch of prep commits, including the terminology, in
r206082, r206083, r206084, r206085, and r206086.

I'm ready to commit the attached patch, which finishes off steps 1-3.
I ran SPEC2000 on ARM64 and saw a slight improvement overall.

Chandler: is it okay with you to move this review to post-commit?
It'll be easier to work on it incrementally in trunk, and I think it's
an improvement over what's there.  Or do you want another pass at this
first via phabricator?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: block-frequency-20140411.patch
Type: application/octet-stream
Size: 140931 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140411/390a410b/attachment.obj>