lkml.org 
[lkml]   [2014]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/3] x86/hash: swap parameters of crc32_u32()
    On 02/24/2014 02:01 PM, H. Peter Anvin wrote:
    > On 02/24/2014 04:51 AM, H. Peter Anvin wrote:
    >> On 02/24/2014 04:41 AM, Jan Beulich wrote:
    >>>>
    >>>> So I'm guessing this hash is deliberately using the CRC32 instruction
    >>>> "backwards", which would actually make sense: an actual CRC is actually
    >>>> a pretty poor hash due to linearity.
    >>
    >> OK, it really is even more confusing than that.
    >>
    >> It does seem like the crc32 instruction really *is* commutative, which
    >> isn't something I would personally have expected at all.
    >>
    >> Given that fact, I suspect the ordering in the DPDK is actually a bug,
    >> and that we should correct the ordering (which I would do at the call
    >> sites because it seems to make the code clearer) because it reduces the
    >> size of the loop by two instructions.
    >>
    >> I guess I should find out how to file a bug report against DPDK too...
    >
    > Looking through the DPDK project git history, it seems that this was a bug introduced when changing from using inline assembly to using intrinsics:
    >
    > static inline uint32_t
    > rte_hash_crc_4byte(uint32_t data, uint32_t init_val)
    > {
    > - asm volatile("crc32 %[data], %[init_val]"
    > - : [init_val]"=r" (init_val)
    > - : [data]"r" (data), "[init_val]" (init_val));
    > - return init_val;
    > + return _mm_crc32_u32(data, init_val);
    > }

    Good point, I also just noticed that in the git blame.

    > The operand order, of course, of the intrinsic being the opposite of AT&T-style assembly.
    >
    > I never expected that the CRC32 operation would be commutative. Very fascinating.

    Indeed.

    > -hpa


    \
     
     \ /
      Last update: 2014-02-24 14:41    [W:2.976 / U:0.148 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site