[Bug default/24965] [dwz] Faster hashing using native insns

[Bug default/24965] [dwz] Faster hashing using native insns

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> and it is unclear why an option is needed.

I've added the option to the patch to make it easy to do the experiment as
mentioned in comment 1. I was not intending to include this in an eventual
patch submission.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[marxin.liska at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

Martin Liška <marxin.liska at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin.liska at gmail dot com

--- Comment #2 from Martin Liška <marxin.liska at gmail dot com> ---
I do welcome the attempt to adapt the crc32 instructions. If I see correctly,
the instructions are available on x86_64 since SSE 4.2. Which should be
available for the vast majority of CPUs.

Note that one can use directly i386 builtins like:
static inline uint64_t _mm_crc32_u64(uint64_t crc, uint64_t value) {
  return __builtin_ia32_crc32qi (crc, value);
}

plus you'll need to use -msse42, or -mcrc32. So I would recommend to use a
start detection of the ISA and then use the instruction.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[marxin.liska at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

--- Comment #5 from Martin Liška <marxin.liska at gmail dot com> ---
(In reply to Jakub Jelinek from comment #4)
> This looks wrong to me.  It will fail to compile on non-x86, fail to run on
> x86 on older hw, and it is unclear why an option is needed.

Sure, it should be conditional for x86_64.
>  It needs to be
> guarded with preprocessor autoconf macros on whether the insns are available
> (assembler handles them) and runtime check whether it can be used or not.
> Formatting glitches too (missing space before ( ).

And the detection should be run-time, so we can use target_clone attribute:
__attribute__((target_clones ("default", "sse42"))) or set guard at the start
up:


#include <config/i386/cpuinfo.h>
...
guard =  ((__cpu_model.__cpu_features[0] & (1 << FEATURE_AVX4_2))

which would be faster as we'll not use indirect calls.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 11975
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11975&action=edit
Add --native-hash

This patch modifies crc32c-intel.patch to make the functionality
off-by-default, and available using a command line option --native-hash.

A first experiment with dwz-ing a cc1 binary, show ~10% speed improvement:
...
$ ../measure/experiment.sh
cmd: ./dwz -L100000000 -l100000000 cc1 -o cc1.dwz
1/10
2/10
3/10
4/10
5/10
6/10
7/10
8/10
9/10
10/10
cmd: ./dwz -L100000000 -l100000000 --native-hash cc1 -o cc1.dwz
1/10
2/10
3/10
4/10
5/10
6/10
7/10
8/10
9/10
10/10
real:
series:  9040 7073 7184 7060 7071 7149 6987 7120 7062 7095
mean:  7284.10 (100%)
stddev:  619.31
series:  6434 6442 6458 6580 6363 6374 6372 6369 6461 6422
mean:  6427.50 (88.24%)
stddev:  65.93
user:
series:  7320 6876 6886 6912 6902 6886 6830 6947 6916 6912
mean:  6938.70 (100%)
stddev:  137.46
series:  6265 6258 6269 6323 6214 6220 6208 6208 6237 6261
mean:  6246.30 (90.02%)
stddev:  36.30
sys:
series:  399 196 292 148 168 252 156 172 144 180
mean:  210.70 (100%)
stddev:  81.39
series:  168 184 188 252 148 152 164 160 220 160
mean:  179.60 (85.24%)
stddev:  33.12
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[jakub at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

Jakub Jelinek <jakub at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at redhat dot com

--- Comment #4 from Jakub Jelinek <jakub at redhat dot com> ---
This looks wrong to me.  It will fail to compile on non-x86, fail to run on x86
on older hw, and it is unclear why an option is needed.  It needs to be guarded
with preprocessor autoconf macros on whether the insns are available (assembler
handles them) and runtime check whether it can be used or not.
Formatting glitches too (missing space before ( ).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[marxin.liska at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

--- Comment #9 from Martin Liška <marxin.liska at gmail dot com> ---
(In reply to Jakub Jelinek from comment #7)
> More portably, you can of course just do cpuid insn yourself and test
> whatever you want.

Yep, there's nice usage:
https://github.com/farsightsec/libmy/blob/master/crc32c-sse42.c

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] New: [dwz] Faster hashing using native insns

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

            Bug ID: 24965
           Summary: [dwz] Faster hashing using native insns
           Product: dwz
           Version: unspecified
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: default
          Assignee: nobody at sourceware dot org
          Reporter: vries at gcc dot gnu.org
                CC: dwz at sourceware dot org
  Target Milestone: ---

Created attachment 11974
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11974&action=edit
crc32c-intel.patch

Martin Liška proposed faster hashing using native insn, like Intel CRC32.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #11975|0                           |1
        is obsolete|                            |

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 11977
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11977&action=edit
Add --native-hash

(In reply to Tom de Vries from comment #1)
> Created attachment 11975 [details]
> Add --native-hash
> 

Oops, wrong patch, now correct.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[jakub at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

--- Comment #7 from Jakub Jelinek <jakub at redhat dot com> ---
More portably, you can of course just do cpuid insn yourself and test whatever
you want.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24965] [dwz] Faster hashing using native insns

[jakub at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24965

--- Comment #6 from Jakub Jelinek <jakub at redhat dot com> ---
(In reply to Martin Liška from comment #5)
> And the detection should be run-time, so we can use target_clone attribute:
> __attribute__((target_clones ("default", "sse42"))) or set guard at the
> start up:
> 
> 
> #include <config/i386/cpuinfo.h>
> ...
> guard =  ((__cpu_model.__cpu_features[0] & (1 << FEATURE_AVX4_2))
> 
> which would be faster as we'll not use indirect calls.

Even that would need guarding with preprocessor macros.  And I think cpuinfo.h
is not an installed header, plus dwz shouldn't be compilable only with recent
versions of gcc and nothing else.

-- 
You are receiving this mail because:
You are on the CC list for the bug.