Re: What exactly is our CRC algorithm?

On 04/02/2015 06:27 PM, Abhijit Menon-Sen wrote:
> At 2015-04-02 17:58:23 +0300, hlinnaka@iki.fi wrote:
>>
>> We're only using inline assembly to force producing SSE 4.2 code, even
>> when -msse4.2 is not used. That feels wrong.
>
> Why? It feels OK to me (and to Andres, per earlier discussions about
> exactly this topic). Doing it this way allows the binary to run on a
> non-SSE4.2 platform (and not use the CRC instructions).

Being able to run on non-SSE4.2 platforms is required, for sure.

> Also, -msse4.2 was added to the compiler later than support for the
> instructions was added to the assembler.

It was added in gcc 4.2. That's good enough for me.

>> We have a buildfarm animal that still uses gcc 2.95.3, which was
>> released in 2001. I don't have a compiler of that vintage to test
>> with, but I assume an old enough assembler would not know about the
>> crc32q instruction and fail to compile.
>
> GCC from <2002 wouldn't support the symbolic operand names in inline
> assembly. binutils from <2007 (IIRC) wouldn't support the assembler
> instructions themselves.
>
> We could work around the latter by using the appropriate sequence of
> bytes. We could work around the former by using the old syntax for
> operands.

I'm OK with not supporting the new instructions when building with an
old compiler/assembler. But the build shouldn't fail with an old
compiler/assembler. Using old syntax or raw bytes just to avoid failing
on an ancient compiler seems ugly.

>> I believe the GCC way to do this would be to put the SSE4.2-specific
>> code into a separate source file, and compile that file with
>> "-msse4.2". And when you compile with -msse4.2, gcc actually also
>> supports the _mm_crc32_u8/u64 intrinsics.
>
> I have no objection to this.
>
> Building only that file with -msse4.2 would resolve the problem of the
> output binary requiring SSE4.2; and the only compilers to be excluded
> are old enough to be uninteresting at least to me personally.
>
> Have you done/are you doing this already, or do you want me to? I could
> use advice on how to add build flags to only one file, since I don't
> know of any precendent for that.

I came up with the attached. The SSE4.2 specific code is now in a
separate file, in src/port/pg_crc32c_sse42.c. The slicing-by-8
implementation is moved to src/port/pg_crc32c_sb8.c, and the function to
choose the implementation at runtime is in src/port/pg_crc32c_choose.c.
How does this look to you?

BTW, we might want to move the "traditional" and "legacy" crc32
implementations out of src/common. They are only used in backend code now.

- Heikki