Based on
98a5a9dc89
with wordsmithing by DRC.
In the AArch64 ABI, as in many others, it's forbidden to read/store data
below the stack pointer. Some SIMD functions were doing just that
(stack pointer misuse) when trying to preserve callee-saved registers,
and this resulted in those registers being restored with incorrect
contents under certain circumstances.
This patch fixes that behavior, and callee-saved registers are now
stored above the stack pointer throughout the function call. The patch
also removes register saving in places where it is unnecessary for this
ABI, or it makes use of unused scratch regiters instead of callee-saved
registers.
Fixes#97. Closes#101.
Refer also to https://bugzilla.redhat.com/show_bug.cgi?id=1368569
In the AArch64 ABI, the high (unused) DWORD of a 32-bit argument's
register is undefined, so it was incorrect to use 64-bit
instructions to transfer a JDIMENSION argument in the 64-bit NEON SIMD
functions. The code worked thus far only because the existing compiler
optimizers weren't smart enough to do anything else with the register in
question, so the upper 32 bits happened to be all zeroes.
The latest builds of Clang/LLVM have a smarter optimizer, and under
certain circumstances, it will attempt to load-combine adjacent 32-bit
integers from one of the libjpeg structures into a single 64-bit integer
and pass that 64-bit integer as a 32-bit argument to one of the SIMD
functions (which is allowed by the ABI, since the upper 32 bits of the
32-bit argument's register are undefined.) This caused the
libjpeg-turbo regression tests to crash.
This patch tries to use the Wn registers whenever possible. Otherwise,
it uses a zero-extend instruction to avoid using the upper 32 bits of
the 64-bit registers, which are not guaranteed to be valid for 32-bit
arguments.
Based on 1fbae13021Closes#91. Refer also to android-ndk/ndk#110 and
https://llvm.org/bugs/show_bug.cgi?id=28393
The IJG convention is to format copyright notices as:
Copyright (C) YYYY, Owner.
We try to maintain this convention for any code that is part of the
libjpeg API library (with the exception of preserving the copyright
notices from Cendio's code verbatim, since those predate
libjpeg-turbo.)
Note that the phrase "All Rights Reserved" is no longer necessary, since
all Buenos Aires Convention signatories signed onto the Berne Convention
in 2000. However, our convention is to retain this phrase for any files
that have a self-contained copyright header but to leave it off of any
files that refer to another file for conditions of distribution and use.
For instance, all of the non-SIMD files in the libjpeg API library refer
to README.ijg, and the copyright message in that file contains "All
Rights Reserved", so it is unnecessary to add it to the individual
files.
The TurboJPEG code retains my preferred formatting convention for
copyright notices, which is based on that of VirtualGL (where the
TurboJPEG API originated.)
The convention used by libjpeg:
type * variable;
is not very common anymore, because it looks too much like
multiplication. Some (particularly C++ programmers) prefer to tuck the
pointer symbol against the type:
type* variable;
to emphasize that a pointer to a type is effectively a new type.
However, this can also be confusing, since defining multiple variables
on the same line would not work properly:
type* variable1, variable2; /* Only variable1 is actually a
pointer. */
This commit reformats the entirety of the libjpeg-turbo code base so
that it uses the same code formatting convention for pointers that the
TurboJPEG API code uses:
type *variable1, *variable2;
This seems to be the most common convention among C programmers, and
it is the convention used by other codec libraries, such as libpng and
libtiff.
Broken by 46ecffa324.
gas-preprocessor.pl and/or the clang assembler apparently don't like
default values in macro arguments, and we need to use a separate const
section for each function (because of our use of adr, also necessitated
by the broken clang assembler.)
... and only if ThunderX is detected. This can be easily expanded later
on to include other CPUs that are known to suffer from slow LD3/ST3, but
it doesn't make sense to disable LD3/ST3 for all non-Android Linux
platforms just because ThunderX is slow.
Full-color compression speedups relative to previous commits:
Cortex-A53 (Nexus 5X), Android, 64-bit: 1.1-13% (avg. 6.0%)
Cortex-A57 (Nexus 5X), Android, 64-bit: 0.0-22% (avg. 6.3%)
Refer to #47 and #50 for discussion
Closes#50
Note that this commit introduces a similar /proc/cpuinfo parser to that
of the ARM32 implementation. It is used to specifically check whether
the code is running on Cavium ThunderX and, if so, disable the ARM64
SIMD Huffman routines (which slow performance by an average of 8% on
that CPU.)
Based on:
a8c282e5e5
There aren't really any best practices to follow here. I tried as best
as I could to adopt a standard that would ease any future maintenance
burdens. The basic tenets of that standard are:
* Assembly instructions always start on Column 5, and operands always
start on Column 21, except:
- The instruction and operand can be indented (usually by 2 spaces)
to indicate a separate instruction stream.
- If the instruction is within an enclosing .if block in a macro,
it should always be indented relative to the .if block.
* Comments are placed with an eye toward readability. There are always
at least 2 spaces between the end of a line of code and the associated
in-line comment. Where it made sense, I tried to line up the comments
in blocks, and some were shifted right to avoid overlap with
neighboring instruction lines. Not an exact science.
* Assembler directives and macros use 2-space indenting rules. .if
blocks are indented relative to the macro, and code within the .if
blocks is indented relative to the .if directive.
* No extraneous spaces between operands. Lining up the operands
vertically did not really improve readability-- personally, I think it
made it worse, since my eye would tend to lose its place in the
uniform columns of characters. Also, code with a lot of vertical
alignment is really hard to maintain, since changing one line could
necessitate changing a bunch of other lines to avoid spoiling the
alignment.
* No extraneous spaces in #defines or other directives. In general, the
only extraneous spaces (other than indenting spaces) are between:
- Instructions and operands
- Operands and in-line comments
This standard should be more or less in keeping with other formatting
standards used within the project.
Decompression speedup relative to libjpeg-turbo 1.4.2 (ISLOW IDCT):
48-core ThunderX (RunAbove ARM Cloud), Linux, 64-bit: 60-113% (avg. 86%)
Cortex-A53 (Nexus 5X), Android, 64-bit: 6.8-27% (avg. 14%)
Cortex-A57 (Nexus 5X), Android, 64-bit: 2.0-14% (avg. 6.8%)
Decompression speedup relative to libjpeg-turbo 1.4.2 (IFAST IDCT):
48-core ThunderX (RunAbove ARM Cloud), Linux, 64-bit: 51-98% (avg. 75%)
Minimal speedup (1-5%) observed on iPhone 5S (Cortex-A7)
NOTE: This commit avoids the st3 instruction for non-Android and
non-Apple builds, which may cause a performance regression against
libjpeg-turbo 1.4.x on ARM64 systems that are running plain Linux.
Since ThunderX is the only platform known to suffer from slow ld3 and
st3 instructions, it is probably better to check for the CPU type
at run time and disable ld3/st3 only if ThunderX is detected.
This commit also enables the use of ld3 on Android platforms, which
should be a safe bet, at least for now. This speeds up compression on
the afore-mentioned Nexus Cortex-A53 by 5.5-19% (avg. 12%) and on the
Nexus Cortex-A57 by 1.2-14% (avg. 6.3%), relative to the previous
commits.
This commit also removes unnecessary macros.
Refer to #52 for discussion.
Closes#52.
Based on:
6bad905034488dd7bf174f4d057c1fd3198afc43
Per @ssvb:
ThunderX is an ARM64 chip that dedicates most of its transistor real
estate to providing 48 cores, so each core is not as fast as a result.
Each core is dual-issue & in-order for scalar instructions and has only
a single-issue half-width NEON unit, so the peak throughput is one
128-bit instruction per 2 cycles. So careful instruction scheduling is
important. Furthermore, ThunderX has an extremely slow implementation
of ld2 and ld3, so this commit implements the equivalent of those
instructions using ld1.
Compression speedup relative to libjpeg-turbo 1.4.2:
48-core ThunderX (RunAbove ARM Cloud), Linux, 64-bit: 58-85% (avg. 74%)
relative to jpeg-6b: 1.75-2.14x (avg. 1.95x)
Refer to #49 and #51 for discussion.
Closes#51.
This commit also wordsmiths the ChangeLog entry (the ARMv8 SIMD
implementation is "complete" only for compression-- it still lacks some
decompression algorithms, as does the ARMv7 implementation.)
Based on:
9405b5fd03
which is based on:
f561944ff7962c8ab21f
This adds 64-bit NEON coverage for all of the algorithms that are
covered by the 32-bit NEON implementation, except for h2v1 (4:2:2) fancy
upsampling (used when decompressing 4:2:2 JPEG images.) It also adds
64-bit NEON SIMD coverage for:
* slow integer forward DCT (compressor)
* h2v2 (4:2:0) downsampling (compressor)
* h2v1 (4:2:2) downsampling (compressor)
which are not covered in the 32-bit implementation.
Compression speedups relative to libjpeg-turbo 1.4.2:
Apple A7 (iPhone 5S), iOS, 64-bit: 113-150% (reported)
48-core ThunderX (RunAbove ARM Cloud), Linux, 64-bit: 2.1-33% (avg. 15%)
Refer to #44 and #49 for discussion
This commit also removes the unnecessary
if (simd_support & JSIMD_ARM_NEON)
statements from the jsimd* algorithm functions. Since the jsimd_can*()
functions check for the existence of NEON, the corresponding algorithm
functions will never be called if NEON isn't available.
Based on:
dcd9d84f10b0d87b811f70cd5c8a493e58d9a064837b19542f73dc43ccc8a82b71a261c1b1188c21305c89284e7f443f99954c2b53b77d
Unified version with fixes:
1004a3cd05
These days, INT32 is a commonly-defined datatype in system headers. We
cannot eliminate the definition of that datatype from jmorecfg.h, since
the INT32 typedef has technically been part of the libjpeg API since
version 5 (1994.) However, using INT32 internally is risky, because the
inclusion of a particular header (Xmd.h, for instance) could change the
definition of INT32 from long to int on 64-bit platforms and thus change
the internal behavior of libjpeg-turbo in unexpected ways (for instance,
failing to correctly set __INT32_IS_ACTUALLY_LONG to match the INT32
typedef-- perhaps as a result of including the wrong version of
jpeglib.h-- could cause libjpeg-turbo to produce incorrect results.)
The library has always been built in environments in which INT32 is
effectively long (on Windows, long is always 32-bit, so effectively it's
the same as int), so it makes sense to turn INT32 into an explicitly
long datatype. This ensures that libjpeg-turbo will always behave
consistently, regardless of the headers included at compile time.
Addresses a concern expressed in #26.
