On Tue, 19 Sep 2017 10:57:05 +0100 Mike Leach mike.leach@linaro.org wrote:
Hi,
On 19 September 2017 at 05:50, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 18 Sep 2017 11:41:29 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
These two patches generated a fair amount of discussion on the mailing list when they were sent and as such I didn't apply them.
But from what I understand people use them when doing autoFDO testing. They may not be perfect but at least provide a base for discussion and further improvement.
My intention is to add them to the perf-opencsd-master branch for the 4.14 cycle. If anyone is categorically against it please get back to me with convincing arguments.
In my tests, the original inject support commit 13c23058bfef "perf tools: new inject capabilitity for CoreSight traces" did not work as advertised: no samples were generated during inject, and continuing to follow the AutoFDO procedure produced a better performing executable only because the -fauto-profile compiler flag enables some optimizations whether the gcov input file is populated or not.
Adding the two patches here on top of that made the autoFDO process produce *poorer* performing executables, because the output gcov data did not correctly represent the true code coverage.
I thus don't think it's a good idea to apply them to the perf-opencsd-master branch, as that gives one the illusion that inject/AutoFDO support works and that they should see an improvement in AutoFDO executable performance as per the intructions in HOWTO.OpenCSD.md appear to demonstrate (when in fact my tests suggest they won't).
Unfortunately having the original patch in the tree already gives the impression that this stuff works.
Indeed. Technically, that can be undone, however.
These patches correct some initial misconceptions regarding the nature of the trace packets so as a basis for further investigation as to why we are not getting good results from the autoFDO flow, I think the three patches are a reasonable starting point.
A starting point for what?
Incomplete feature development doesn't belong on master branches.
I do think however, that it would be worthwhile modifying HOWTO.md, and / or add a README-autoFDO.md that makes it clear that these patches are available on an "experimental" basis and will not be upstreamed until all are convinced that they have been improved to the point that we get consistent results in improving executables using the autoFDO flow.
Better even to migrate the original patch and other two to their own experimental branch, or tree, even.
We've discussed off-list the creation of a separate tree to maintain the decoder library project, and a separate tree to maintain the perf to-be-upstreamed material, this is a chance to have these changes be done in conjunction with that effort.
Kim