On Mon, Aug 24, 2015 at 2:09 PM, Pekka Paalanen <ppaala...@gmail.com> wrote: > On Sat, 22 Aug 2015 13:14:08 -0700 > Matt Turner <matts...@gmail.com> wrote: > >> On Thu, Aug 20, 2015 at 6:58 AM, Pekka Paalanen <ppaala...@gmail.com> wrote: >> > From: Ben Avison <bavi...@riscosopen.org> >> > >> > This is a C fast path, useful for reference or for platforms that don't >> > have their own fast path for this operation. >> > >> > This new fast path is initially disabled by putting the entries in the >> > lookup table after the sentinel. The compiler cannot tell the new code >> > is not used, so it cannot eliminate the code. Also the lookup table size >> > will include the new fast path. When the follow-up patch then enables >> > the new fast path, the binary layout (alignments, size, etc.) will stay >> > the same compared to the disabled case. >> > >> > Keeping the binary layout identical is important for benchmarking on >> > Raspberry Pi 1. The addresses at which functions are loaded will have a >> > significant impact on benchmark results, causing unexpected performance >> > changes. Keeping all function addresses the same across the patch >> > enabling a new fast path improves the reliability of benchmarks. >> > >> > Benchmark results are included in the patch enabling this fast path. >> > >> > [Pekka: disabled the fast path, commit message] >> > Signed-off-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> >> >> I don't care strongly, but I might just squash 1+2, 3+4 together and >> make a mention in the commit message of exactly what the benchmark >> numbers are comparing. > > Hi Matt, > > yes, that's always a possibility, but then I need to explain what the > benchmarked code revisions looked like which is prone to > misunderstanding. So personally I would prefer to land exactly the > revisions of the code that were tested/benchmarked, so that posterity > can (at least in theory) repeat the benchmarks if needed. Or has that > little value? > > Maybe I should elaborate on how to iterate benchmarks in the "enable" > patches. > > To me this has documentary value, also in educating others on the > quirks of rpi1. I don't generally intend to use this style for anything > but rpi1 specific patches. > > Does keeping the patches split make reviewing harder? > > I could also keep these 4 as is and squash the future patches? > > > Thanks, > pq > > _______________________________________________ > Pixman mailing list > Pixman@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/pixman >
Hi, Although I would generally agree with Matt, I believe that due to rpi-1 quirks, Pekka's approach is correct in this specific case. I think that reproducing the exact results/benchmarks has more than enough value to justify the separate patches this time. Anyway, that's just my 2 cents. Oded _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman