[18.10] zlib: improve crc32 performance on P8

Bug #1742941 reported by bugproxy on 2018-01-12
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
The Ubuntu-power-systems project
Low
Canonical Foundations Team
zlib (Ubuntu)
Low
Canonical Foundations Team

Bug Description

Calculate the checksum of data that is 16 byte aligned and a multiple of 16 bytes.

The first step is to reduce it to 1024 bits. We do this in 8 parallel
 chunks in order to mask the latency of the vpmsum instructions. If we
 have more than 32 kB of data to checksum we repeat this step multiple
 times, passing in the previous 1024 bits.

 The next step is to reduce the 1024 bits to 64 bits. This step adds
 32 bits of 0s to the end - this matches what a CRC does. We just
 calculate constants that land the data in this 32 bits.

 We then use fixed point Barrett reduction to compute a mod n over GF(2)
 for n = CRC using POWER8 instructions. We use x = 32.

 http://en.wikipedia.org/wiki/Barrett_reduction

Default Comment by Bridge

tags: added: architecture-ppc64le bugnameltc-136495 severity-low targetmilestone-inin1804
Changed in ubuntu:
assignee: nobody → Ubuntu on IBM Power Systems Bug Triage (ubuntu-power-triage)
affects: ubuntu → zlib (Ubuntu)

------- Comment From <email address hidden> 2018-01-12 08:22 EDT-------
>'ve created the following which hopefully contains all the features
> required to be upstream friendly.
>
> https://github.com/grooverdan/zlib/commits/power_crc32_c_version
>
> Most importantly compared to the previous PR, it contains the right crc32
> constants.
>
> If you see anything that upstream/distros or anyone else might nit about
> then please tell me (on github or here). The upstream isn't very active so
> presenting something idea on day one is my goal.
>
> Note 1: no clang currently due to...
>
> Note 1a: crc32_vpmsum: __builtin_pack_vector_int128 and
> __builtin_crypto_vpmsumw/__builtin_crypto_vpmsumd - need to review
> https://github.com/racardoso/crc32-vpmsum/commit/
> 97210b9188916eb46377b5eb927ae337948bf016 properly (sorry Rogerio, from
> memory it needs to compile fail for clang BE rather than generate wrong
> results).
>
> Note 1b: clang - no __builtin_cpu_supports (
> https://bugs.llvm.org/show_bug.cgi?id=35898 ) so I might fall back to
> getauxval
>
>
> Is there an better define than __powerpc__ to check in configure to detect
> Power8+ only?
>
> Other suggestions welcome.

Changed in ubuntu-power-systems:
importance: Undecided → Low
assignee: nobody → David Britton (davidpbritton)
tags: added: triage-g
Manoj Iyer (manjo) on 2018-01-16
Changed in zlib (Ubuntu):
assignee: Ubuntu on IBM Power Systems Bug Triage (ubuntu-power-triage) → David Britton (davidpbritton)
importance: Undecided → Low
Changed in ubuntu-power-systems:
status: New → Triaged
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2018-01-17 22:56 EDT-------
code submitted upstream: https://github.com/madler/zlib/pull/335

Changed in zlib (Ubuntu):
assignee: David Britton (davidpbritton) → Patricia Gaughen (gaughen)
Changed in ubuntu-power-systems:
assignee: David Britton (davidpbritton) → nobody
Changed in zlib (Ubuntu):
assignee: Patricia Gaughen (gaughen) → Canonical Foundations Team (canonical-foundations)

this is not yet committed upstream (2018-01-30).

Manoj Iyer (manjo) on 2018-02-05
Changed in ubuntu-power-systems:
status: Triaged → Incomplete
Manoj Iyer (manjo) on 2018-02-12
Changed in ubuntu-power-systems:
assignee: nobody → Canonical Foundations Team (canonical-foundations)
Manoj Iyer (manjo) wrote :

IBM, Can you please update this bug when this patch lands upstream?

------- Comment From <email address hidden> 2018-04-02 08:58 EDT-------
(In reply to comment #45)
> IBM, Can you please update this bug when this patch lands upstream?

Yes, but I do not think we can make 18.04 anymore. Deferring to 18.10.

tags: added: targetmilestone-inin1810
removed: targetmilestone-inin1804
Manoj Iyer (manjo) on 2018-04-05
summary: - zlib: improve crc32 performance on P8
+ [18.10] zlib: improve crc32 performance on P8
Manoj Iyer (manjo) wrote :

IBM, 18.10 feature freeze is on the 23rd of Aug, any news on the upstream status of this patch?

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2018-08-20 14:53 EDT-------
> IBM, 18.10 feature freeze is on the 23rd of Aug, any news on the upstream
> status of this patch?

The patch has been waiting for the zlib maintainer since Feb. 6th.
Likewise for all the pull requests submitted since January: https://github.com/madler/zlib/pulls?q=is%3Aopen+is%3Apr

Canonical, would it be possible to carry this patchset out-of-tree?

Manoj Iyer (manjo) wrote :

Looks like this patch is still waiting to be merged upstream, looking at this link: https://github.com/madler/zlib/pull/335 Could you please advice if this is being considered to be merged upstream by the maintainer?

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2019-04-08 18:59 EDT-------
Significant background talks are taking place with Mark Adler about the maintenance of zlib for arch specific performance.

Lets see what happens in the next 3 months.

Changed in zlib (Ubuntu):
status: New → Incomplete
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.