New issue
Advanced search Search tips

Issue 810125 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 3
Type: Bug

Blocked on:
issue 709716
issue 848897
issue 863257

Blocking:
issue 709707



Sign in to add a comment

[zlib][arm]: enable crc32 for missing platforms

Project Member Reported by cavalcantii@chromium.org, Feb 7 2018

Issue description

As soon we land  issue #709716 , we got start investigation to enable the optimization for the following OSes:
a) Chrome OS (CrOS): it seems that they use some scripts for building that may bypass/add compilation flags that messes up with the required flags for using ARMv8-a crypto crc32 instruction.

b) Fuchsia: support for checking ARM processor features just landed (https://fuchsia-review.googlesource.com/c/zircon/+/115943), it would be nice to also enable the optimization for them.

 
Blockedon: 709716
Blocking: 709707

Comment 2 by noel@chromium.org, Feb 9 2018

Cc: noel@chromium.org
Thanks for filing this bug.  For the fuchia zircon-os case, I'm chatting with the developers on https://fuchsia.atlassian.net/browse/ZX-1552 as an FYI.
Sounds good, thanks for getting them to add the missing syscall.

I tried to access the issue but I guess is googlers only (got "Your email address cavalcantii@chromium.org doesn't have access to fuchsia.atlassian.net").

Comment 4 by noel@chromium.org, Feb 10 2018

Same for me, @chromium.org failed, @google.com worked.  The current summary is that after https://fuchsia-review.googlesource.com/c/zircon/+/115943 landed it would soon roll into chromium.  

I asked the fuchsia/zircon-os folks if they were OK with us using it in chromium.
Answer: not yet, they're changing the API to provide for future-proofing.

Recall the HWPCAP / HWCAP2 issue in the linux kernel for ARM, where they were about to ran out of feature bits for getauxv() HWPCAP, and had to introduce HWCAP2 to enable support 32bit ARM binaries running in __AARCH64__.   Worked, but somewhat confusing.

The fuchsia/zircon-os folks are looking for something that would be future-proof, and work for their supported targets (ARM64 and Intel X64).  So they are changing the API [1].  It'll look something like this for a user-space program wanting to know about ARM64 CRC32 support for example ...

#include <zircon/features.h>

extern int arm64_crc32 = 0;

void detect_cpu_features() {
  uint32_t cpu_features = 0;
  zx_system_get_features(ZX_FEATURE_KIND_CPU, &cpu_features);
  arm64_crc32 = cpu_features & ZX_ARM64_FEATURE_ISA_CRC32;
}

[1] https://fuchsia-review.googlesource.com/c/zircon/+/120576

Note that zircon targets 64-bit processors only: Intel x64, ARM64.

Comment 5 by noel@chromium.org, Feb 10 2018

Anyho, once they a ready / done with this API design, they will ping me, and we can then proceed to use it in chrome third_party/zlib.

Cc: cblume@chromium.org

Comment 7 by noel@chromium.org, Feb 15 2018

New from Aaron over in fuchsia.  Patch [1] submitted

[1] https://fuchsia-review.googlesource.com/c/zircon/+/120576

"kind is similar but distinct to leaf. In particular I think Brian's idea was to be able to serve other, non-CPU feature bits as well this way."

re: the code I posted in #4

"That code should work. You might want to catch the return value to check for ZX_ERR_NOT_SUPPORTED. As written, it works today since features is untouched, but that's not guaranteed by the documentation."

Comment 8 by noel@chromium.org, Feb 16 2018

Blockedon: 848897
Blockedon: 863257
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b3f32ad27fdd569e39289f241043f7111da78fa

commit 9b3f32ad27fdd569e39289f241043f7111da78fa
Author: Tiancong Wang <tcwang@google.com>
Date: Thu Dec 13 04:20:08 2018

Enabling crc32 ARM instruction in zlib for ChromeOS.

Using the ARM crc32 instruction can boost the speed of gzip content
decompression, which can improve performances in cases where lots of
webpages are served using 'content-encoding: gzip'.

Expected to have ~50% decompression speed improvement on elm.

Bug:  848897 , 810125
Change-Id: I9f090950209e6a68271c6926700b7335e14c7cbf
Reviewed-on: https://chromium-review.googlesource.com/c/1372548
Commit-Queue: Adenilson Cavalcanti <cavalcantii@chromium.org>
Reviewed-by: Adenilson Cavalcanti <cavalcantii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616210}
[modify] https://crrev.com/9b3f32ad27fdd569e39289f241043f7111da78fa/third_party/zlib/BUILD.gn

Sign in to add a comment