Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libpng18] reworked SIMD code #589

Closed
wants to merge 0 commits into from

Conversation

jbowler
Copy link
Contributor

@jbowler jbowler commented Sep 13, 2024

This is the rework of the hardware (SIMD) support based on @csparker247's
pull request #575.

It incorporates that request rebased on the current master; the request
should be pulled first.

This passes cross and hosted build checks as follows:

aarch64 configure make all SUCCESS
aarch64 cmake make all SUCCESS
amd64 configure make all SUCCESS
amd64 cmake make all SUCCESS
armv7a configure make all SUCCESS
armv7a cmake make all SUCCESS
armv7a-neon configure make all SUCCESS
armv7a-neon cmake make all SUCCESS
clang cmake make all SUCCESS
mips configure make all SUCCESS
mips cmake make all SUCCESS
riscv64 configure make all SUCCESS
riscv64 cmake make all SUCCESS
clang configure make check SUCCESS
clang cmake make test SUCCESS

  • cmake: Remove architecture-specific checks for hardware optimization
  • Full compile time support for target-specific code
  • MIPS support clean-up

@jbowler jbowler changed the base branch from libpng16 to master September 13, 2024 01:04
@jbowler jbowler changed the base branch from master to libpng18 September 14, 2024 23:57
@jbowler jbowler force-pushed the target-specific-code branch 2 times, most recently from e390d1d to b8ebfd2 Compare September 15, 2024 15:11
@jbowler jbowler force-pushed the target-specific-code branch from b8ebfd2 to b96d06b Compare September 16, 2024 19:07
@jbowler
Copy link
Contributor Author

jbowler commented Sep 16, 2024

I've squashed the changes into one commit and removed the file moves. Firstly there is a good chance of merge conflicts with file moves and the merge resolution normally ends up dropping changes accidentally. Secondly I'm not sure they were correct.

What I favor, after all the pending pull requests have been resolved, is a directory tree structure where all the target-specific code is in one directory. Possibly a subdirectory called "arch" with subdirectories below that for each individual architecture (i.e the first part of a CHOST tuple). This kind of structure:

arch/<architecture>/<instruction set>/<implementation>

Where the instruction set subdirectory is only necessary if there are multiple implementations for different instruction sets (as on MIPS). In fact it doesn't matter much below the architecture level because my idea is that third party maintainers store third party code in there (licensed under the libpng license of course) and never alter anything above that level. When the implementation is satisfactory the libpng maintainer adds the one required line to pngtarget.h as in the current code; no changes are required (or permitted :-) in pngsimd.c

@jbowler jbowler force-pushed the target-specific-code branch 2 times, most recently from 24944b5 to 9dcf5dd Compare September 18, 2024 19:30
@jbowler
Copy link
Contributor Author

jbowler commented Sep 18, 2024

The Lint changes did not pick up the errors detected by editorconfig-checker. I had to run editorconfig-checker directly to detect them.. Something in the recent changes (today) has broken the cmakeAMD64 builds (make check/test).

@jbowler
Copy link
Contributor Author

jbowler commented Sep 18, 2024

No; it's the 10700 problem, it was always there in any make check run.

@jbowler jbowler force-pushed the target-specific-code branch from 9dcf5dd to c62247d Compare September 18, 2024 20:00
@jbowler
Copy link
Contributor Author

jbowler commented Sep 18, 2024

Rebased on #596. Make check and make test now pass on AMD64, ararch64, armv7a (with and without -mfpu=neon), mips and riscv64 (no target specific code for that yet) all now build. aarch64 and armv7a were passing the tests too insofar as I could run them (a couple of days ago, before the latest merge). MIPS32 builds (no target specific code) but I can't manage to make a MIPS64 compiler; all my crossdevs fail at the last stage of building gcc.

clang was tested too on AMD64 (making clang cross compilers appears to be a little more difficult.)

@jbowler
Copy link
Contributor Author

jbowler commented Sep 18, 2024

Rebased, all tests passed

@jbowler jbowler force-pushed the target-specific-code branch from c62247d to 4716d87 Compare September 21, 2024 15:24
@jbowler
Copy link
Contributor Author

jbowler commented Sep 22, 2024

This should wait because it is development rather than a bug fix. It's not clear how that (development) will be done at this point.

@jbowler jbowler closed this Sep 22, 2024
@jbowler jbowler force-pushed the target-specific-code branch from 4716d87 to 626761b Compare September 22, 2024 08:27
@jbowler jbowler deleted the target-specific-code branch September 22, 2024 08:30
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.

1 participant