Skip to content

[v2] Base cross-arch support#2

Open
daxtens wants to merge 14 commits into
gbtucker:masterfrom
daxtens:wip
Open

[v2] Base cross-arch support#2
daxtens wants to merge 14 commits into
gbtucker:masterfrom
daxtens:wip

Conversation

@daxtens
Copy link
Copy Markdown

@daxtens daxtens commented Mar 20, 2017

As discussed in #1, I have:

  • got rid of all arch-specific bits of the public API, and all arch specific tests
  • added copyright headers
  • squashed the CRC duplication in igzip.

In addition, I wanted to make sure I hadn't broken 32-bit, which turned out to be a bit more complex than I thought as it didn't build out of the box with autotools. There are patches to:

  • enable 32 bit builds with autotools (patch 2)
  • use the base versions for crc32 and igzip, which don't seem to have working 32 bit versions (correct me if I'm wrong)
  • added a new dispatcher to dispatch the SSE3 functions on for RAID on 32-bit systems.

I have enabled 32-bit Linux and 64-bit OS X tests in the final patch as well.

I have verified that 'make test check perf' works on 64 and 32 bit x86, and ppc64le. I will add any fixes to compile on Windows tomorrow.

daxtens added 3 commits March 20, 2017 21:16
This allows builds on non-x86 platforms to get past ./configure

Signed-off-by: Daniel Axtens <dja@axtens.net>
I have been testing with:
    ./configure --host=i686-pc-linux-gnu CFLAGS="-m32 -g -O2"

Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Daniel Axtens <dja@axtens.net>
@daxtens
Copy link
Copy Markdown
Author

daxtens commented Mar 20, 2017

ergh, turns out this is actually broken on x86_64! Fix to come.

@daxtens
Copy link
Copy Markdown
Author

daxtens commented Mar 20, 2017

and it turns out that gf_vect_mad_avx2 returns different results to gf_vect_mad_{avx,sse}, which is what's causing the problem.

@daxtens
Copy link
Copy Markdown
Author

daxtens commented Mar 21, 2017

Reported upstream: intel#19

daxtens added 10 commits March 21, 2017 11:52
 - remove instruction-type specific tests
 - make gf_vect_mad_{test,perf} generic, not sse-specific

Signed-off-by: Daniel Axtens <dja@axtens.net>
ec_highlevel_func.c has code for SSE/AVX etc, and the headers have
prototypes for them as well.

We want the API to only contain arch-independent functions.

Break the highlevel x86 functions into their own file.
Move the remaining function into ec_base.c, delete ec_highlevel_func.c
Remove the x86-specific prototypes from the public headers.
Create a new, private header file with the prototypes.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Calling the base functions directly from C in a base aliases file
shows that they don't line up. This isn't normally seen because
they are invoked from asm.

Make the types line up, do simple resulting cleanup.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Use this for platform generic code and 32-bit code.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Daniel Axtens <dja@axtens.net>
This contains:
 - standard source grouping fixes

 - standard base aliases, used for both generic and 32-bit.

 - expose the CrcData table to C with crc_data.c.
   I have removed the old asm file and verified that things
   still work when built with autotools.

 - only include the xmm/imm intrinsics headers on x86.

Signed-off-by: Daniel Axtens <dja@axtens.net>
 - test on 32 bit Linux
 - test on OS X (yasm only, nasm doesn't work)
 - add the 'test' target as well as 'check'

Signed-off-by: Daniel Axtens <dja@axtens.net>
@daxtens
Copy link
Copy Markdown
Author

daxtens commented Mar 21, 2017

Ok, that should be good to go now :)

@gbtucker
Copy link
Copy Markdown
Owner

The fixes look ok. I have all the base aliases already and some of the x86 fixes but the others should apply cleanly. We have a lot of changes to compression coming and I'll ask you then to rebase on that.

When I renamed ec_highlevel_func.c to ec_highlevel_x86.c, I
didn't update this makefile.

Tested on a Windows 10 developer VM using the MS compiler.

Signed-off-by: Daniel Axtens <dja@axtens.net>
@daxtens
Copy link
Copy Markdown
Author

daxtens commented Mar 23, 2017

Excellent - let me know once they land and I'll send you a v3.

@gbtucker gbtucker force-pushed the master branch 5 times, most recently from 7ecb66c to 2315944 Compare September 20, 2017 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants