Re: [PATCH 1/3] powerpc: remove power6 wcscpy optimization

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

On Mon, Mar 04 2019, Adhemerval Zanella wrote:
> This patch removes the power6 wcscpy optimization.  It is rarely
> int real word codo, the optimizations can be potentially implemented
> in generic code (which would also benefit not only powerpc), and
> the power6 and power7 resulting binary are essentially the same.

Although I agree that optimizations could be implemented in generic
code, and that it might benefit other architectures, I tested this patch
and it causes large performance regressions in benchtests, when ran on
Power8 and Power9 machines (Power 7 numbers were OK).  Based on that
result (which I shall present and discuss later in this message), I
think this patch should not be committed to master.

I am aware that this patch has the additional benefit of easing code
maintenance, however, the performance impact on Power would be seen
immediately, while the benefit of an optimization to the generic
algorithm would need more time to implement.  Thus, I suggest that this
patch be delayed until we have such implementation.


Now, regarding the results, I have attached a few spreadsheets, each
containing the raw data collected from 'make bench' runs on different
machines, as well as comparisons between the master branch and another
branch with this patch applied.

  attachment/wcscpy-LE-P9.ods: Runs on a Power 9, little-endian machine.
  attachment/wcscpy-LE-P8.ods: Runs on a Power 8, little-endian machine.
  attachment/wcscpy-BE-P8.ods: Runs on a Power 8, big-endian machine.
  attachment/wcscpy-BE-P7.ods: Runs on a Power 7, big-endian machine.

In each of these files, each sheet is named according to the data it
contains: for instance, 'master - cpu=p8' contains the raw data
collected from a build of the master branch, which has been configured
with --with-cpu=power8.  Likewise, 'patched - cpu=p8' contains similar
data, but from a build of a branch with this patch applied.  Finally, a
sheet named 'compare p8' contains the performance comparison between the
two.  In such 'compare pN' sheets, I added a graph of the performance
comparison of the two, in which the data is sorted from worst to best
case, so that we can have a visual assessment of how good or bad the
change is for the whole set of inputs tested (I believe that this kind
of plot is common practice, but I'm not sure, hence this explanation).

In wcscpy-BE-P8.ods, wcscpy-LE-P8.ods, and wcscpy-LE-P9.ods, we can see
that, for allmost all inputs, there is a performance regression (dots
below 0% are regressions).  On wcscpy-BE-P7.ods, there are more
performance gains than regressions, and I would be OK with this patch
being applied to master, if it weren't for the P8 and P9 results.

PS: I have not yet tested the other patches in this patch set.  This
message is only about this specific patch.

Attachment: wcscpy-LE-P9.ods
Description: application/vnd.oasis.opendocument.spreadsheet

Attachment: wcscpy-LE-P8.ods
Description: application/vnd.oasis.opendocument.spreadsheet

Attachment: wcscpy-BE-P8.ods
Description: application/vnd.oasis.opendocument.spreadsheet

Attachment: wcscpy-BE-P7.ods
Description: application/vnd.oasis.opendocument.spreadsheet