Skip to content

Commit

Permalink
SIMD/x86: Initialize simd_support before every use
Browse files Browse the repository at this point in the history 
As long as a libjpeg instance is only used by one thread at a time, a
program is technically within its rights to call jpeg_start_*compress()
in one thread and jpeg_(read|write)_*(), with the same libjpeg instance,
in a second thread.  However, because the various jsimd_can*() functions
are called within the body of jpeg_start_*compress() and simd_support is
now thread-local (due to f579cc1), that
led to a situation in which simd_support was initialized in the first
thread but not the second.  The uninitialized value of simd_support is
0xFFFFFFFF, which the second thread interpreted to mean that it could
use any instruction set, and when it attempted to use AVX2 instructions
on a CPU that didn't support them, an illegal instruction error
occurred.

This issue was known to affect libvips.

This commit modifies the i386 and x86-64 SIMD dispatchers so that the
various jsimd_*() functions always call init_simd(), if simd_support is
uninitialized, prior to dispatching based on the value of simd_support.
Note that the other SIMD dispatchers don't need this, because only the
x86 SIMD extensions currently support multiple instruction sets.

This patch has been verified to be performance-neutral to within
+/- 0.4% with 32-bit and 64-bit code running on a 2.8 GHz Intel Xeon
W3530 and a 3.6 GHz Intel Xeon W2123.

Fixes #649
  • Loading branch information
@dcommander
dcommander committed Feb 2, 2023
1 parent 89ceac8 commit 4e028ec
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 2 deletions.
71 changes: 70 additions & 1 deletion simd/i386/jsimd.c
View file
Expand Up @@ -2,7 +2,7 @@
* jsimd_i386.c
*
* Copyright 2009 Pierre Ossman <ossman@cendio.se> for Cendio AB
* Copyright (C) 2009-2011, 2013-2014, 2016, 2018, 2022, D. R. Commander.
* Copyright (C) 2009-2011, 2013-2014, 2016, 2018, 2022-2023, D. R. Commander.
* Copyright (C) 2015-2016, 2018, 2022, Matthieu Darbois.
*
* Based on the x86 SIMD extension for IJG JPEG library,
Expand Down Expand Up @@ -158,6 +158,9 @@ jsimd_rgb_ycc_convert(j_compress_ptr cinfo, JSAMPARRAY input_buf,
void (*sse2fct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);
void (*mmxfct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);

if (simd_support == ~0U)
init_simd();

switch (cinfo->in_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_extrgb_ycc_convert_avx2;
Expand Down Expand Up @@ -217,6 +220,9 @@ jsimd_rgb_gray_convert(j_compress_ptr cinfo, JSAMPARRAY input_buf,
void (*sse2fct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);
void (*mmxfct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);

if (simd_support == ~0U)
init_simd();

switch (cinfo->in_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_extrgb_gray_convert_avx2;
Expand Down Expand Up @@ -276,6 +282,9 @@ jsimd_ycc_rgb_convert(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
void (*sse2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY, int);
void (*mmxfct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY, int);

if (simd_support == ~0U)
init_simd();

switch (cinfo->out_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_ycc_extrgb_convert_avx2;
Expand Down Expand Up @@ -379,6 +388,9 @@ GLOBAL(void)
jsimd_h2v2_downsample(j_compress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY output_data)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v2_downsample_avx2(cinfo->image_width, cinfo->max_v_samp_factor,
compptr->v_samp_factor,
Expand All @@ -399,6 +411,9 @@ GLOBAL(void)
jsimd_h2v1_downsample(j_compress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY output_data)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v1_downsample_avx2(cinfo->image_width, cinfo->max_v_samp_factor,
compptr->v_samp_factor,
Expand Down Expand Up @@ -461,6 +476,9 @@ GLOBAL(void)
jsimd_h2v2_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v2_upsample_avx2(cinfo->max_v_samp_factor, cinfo->output_width,
input_data, output_data_ptr);
Expand All @@ -476,6 +494,9 @@ GLOBAL(void)
jsimd_h2v1_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v1_upsample_avx2(cinfo->max_v_samp_factor, cinfo->output_width,
input_data, output_data_ptr);
Expand Down Expand Up @@ -537,6 +558,9 @@ GLOBAL(void)
jsimd_h2v2_fancy_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v2_fancy_upsample_avx2(cinfo->max_v_samp_factor,
compptr->downsampled_width, input_data,
Expand All @@ -555,6 +579,9 @@ GLOBAL(void)
jsimd_h2v1_fancy_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v1_fancy_upsample_avx2(cinfo->max_v_samp_factor,
compptr->downsampled_width, input_data,
Expand Down Expand Up @@ -623,6 +650,9 @@ jsimd_h2v2_merged_upsample(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
void (*sse2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);
void (*mmxfct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);

if (simd_support == ~0U)
init_simd();

switch (cinfo->out_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_h2v2_extrgb_merged_upsample_avx2;
Expand Down Expand Up @@ -681,6 +711,9 @@ jsimd_h2v1_merged_upsample(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
void (*sse2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);
void (*mmxfct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);

if (simd_support == ~0U)
init_simd();

switch (cinfo->out_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_h2v1_extrgb_merged_upsample_avx2;
Expand Down Expand Up @@ -785,6 +818,9 @@ GLOBAL(void)
jsimd_convsamp(JSAMPARRAY sample_data, JDIMENSION start_col,
DCTELEM *workspace)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_convsamp_avx2(sample_data, start_col, workspace);
else if (simd_support & JSIMD_SSE2)
Expand All @@ -797,6 +833,9 @@ GLOBAL(void)
jsimd_convsamp_float(JSAMPARRAY sample_data, JDIMENSION start_col,
FAST_FLOAT *workspace)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_SSE2)
jsimd_convsamp_float_sse2(sample_data, start_col, workspace);
else if (simd_support & JSIMD_SSE)
Expand Down Expand Up @@ -867,6 +906,9 @@ jsimd_can_fdct_float(void)
GLOBAL(void)
jsimd_fdct_islow(DCTELEM *data)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_fdct_islow_avx2(data);
else if (simd_support & JSIMD_SSE2)
Expand All @@ -878,6 +920,9 @@ jsimd_fdct_islow(DCTELEM *data)
GLOBAL(void)
jsimd_fdct_ifast(DCTELEM *data)
{
if (simd_support == ~0U)
init_simd();

if ((simd_support & JSIMD_SSE2) && IS_ALIGNED_SSE(jconst_fdct_islow_sse2))
jsimd_fdct_ifast_sse2(data);
else
Expand All @@ -887,6 +932,9 @@ jsimd_fdct_ifast(DCTELEM *data)
GLOBAL(void)
jsimd_fdct_float(FAST_FLOAT *data)
{
if (simd_support == ~0U)
init_simd();

if ((simd_support & JSIMD_SSE) && IS_ALIGNED_SSE(jconst_fdct_float_sse))
jsimd_fdct_float_sse(data);
else if (simd_support & JSIMD_3DNOW)
Expand Down Expand Up @@ -942,6 +990,9 @@ jsimd_can_quantize_float(void)
GLOBAL(void)
jsimd_quantize(JCOEFPTR coef_block, DCTELEM *divisors, DCTELEM *workspace)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_quantize_avx2(coef_block, divisors, workspace);
else if (simd_support & JSIMD_SSE2)
Expand All @@ -954,6 +1005,9 @@ GLOBAL(void)
jsimd_quantize_float(JCOEFPTR coef_block, FAST_FLOAT *divisors,
FAST_FLOAT *workspace)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_SSE2)
jsimd_quantize_float_sse2(coef_block, divisors, workspace);
else if (simd_support & JSIMD_SSE)
Expand Down Expand Up @@ -1017,6 +1071,9 @@ jsimd_idct_2x2(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JCOEFPTR coef_block, JSAMPARRAY output_buf,
JDIMENSION output_col)
{
if (simd_support == ~0U)
init_simd();

if ((simd_support & JSIMD_SSE2) && IS_ALIGNED_SSE(jconst_idct_red_sse2))
jsimd_idct_2x2_sse2(compptr->dct_table, coef_block, output_buf,
output_col);
Expand All @@ -1029,6 +1086,9 @@ jsimd_idct_4x4(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JCOEFPTR coef_block, JSAMPARRAY output_buf,
JDIMENSION output_col)
{
if (simd_support == ~0U)
init_simd();

if ((simd_support & JSIMD_SSE2) && IS_ALIGNED_SSE(jconst_idct_red_sse2))
jsimd_idct_4x4_sse2(compptr->dct_table, coef_block, output_buf,
output_col);
Expand Down Expand Up @@ -1123,6 +1183,9 @@ jsimd_idct_islow(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JCOEFPTR coef_block, JSAMPARRAY output_buf,
JDIMENSION output_col)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_idct_islow_avx2(compptr->dct_table, coef_block, output_buf,
output_col);
Expand All @@ -1139,6 +1202,9 @@ jsimd_idct_ifast(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JCOEFPTR coef_block, JSAMPARRAY output_buf,
JDIMENSION output_col)
{
if (simd_support == ~0U)
init_simd();

if ((simd_support & JSIMD_SSE2) && IS_ALIGNED_SSE(jconst_idct_ifast_sse2))
jsimd_idct_ifast_sse2(compptr->dct_table, coef_block, output_buf,
output_col);
Expand All @@ -1152,6 +1218,9 @@ jsimd_idct_float(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JCOEFPTR coef_block, JSAMPARRAY output_buf,
JDIMENSION output_col)
{
if (simd_support == ~0U)
init_simd();

if ((simd_support & JSIMD_SSE2) && IS_ALIGNED_SSE(jconst_idct_float_sse2))
jsimd_idct_float_sse2(compptr->dct_table, coef_block, output_buf,
output_col);
Expand Down
47 changes: 46 additions & 1 deletion simd/x86_64/jsimd.c
View file
Expand Up @@ -2,7 +2,7 @@
* jsimd_x86_64.c
*
* Copyright 2009 Pierre Ossman <ossman@cendio.se> for Cendio AB
* Copyright (C) 2009-2011, 2014, 2016, 2018, 2022, D. R. Commander.
* Copyright (C) 2009-2011, 2014, 2016, 2018, 2022-2023, D. R. Commander.
* Copyright (C) 2015-2016, 2018, 2022, Matthieu Darbois.
*
* Based on the x86 SIMD extension for IJG JPEG library,
Expand Down Expand Up @@ -145,6 +145,9 @@ jsimd_rgb_ycc_convert(j_compress_ptr cinfo, JSAMPARRAY input_buf,
void (*avx2fct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);
void (*sse2fct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);

if (simd_support == ~0U)
init_simd();

switch (cinfo->in_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_extrgb_ycc_convert_avx2;
Expand Down Expand Up @@ -194,6 +197,9 @@ jsimd_rgb_gray_convert(j_compress_ptr cinfo, JSAMPARRAY input_buf,
void (*avx2fct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);
void (*sse2fct) (JDIMENSION, JSAMPARRAY, JSAMPIMAGE, JDIMENSION, int);

if (simd_support == ~0U)
init_simd();

switch (cinfo->in_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_extrgb_gray_convert_avx2;
Expand Down Expand Up @@ -243,6 +249,9 @@ jsimd_ycc_rgb_convert(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
void (*avx2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY, int);
void (*sse2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY, int);

if (simd_support == ~0U)
init_simd();

switch (cinfo->out_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_ycc_extrgb_convert_avx2;
Expand Down Expand Up @@ -333,6 +342,9 @@ GLOBAL(void)
jsimd_h2v2_downsample(j_compress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY output_data)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v2_downsample_avx2(cinfo->image_width, cinfo->max_v_samp_factor,
compptr->v_samp_factor,
Expand All @@ -349,6 +361,9 @@ GLOBAL(void)
jsimd_h2v1_downsample(j_compress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY output_data)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v1_downsample_avx2(cinfo->image_width, cinfo->max_v_samp_factor,
compptr->v_samp_factor,
Expand Down Expand Up @@ -403,6 +418,9 @@ GLOBAL(void)
jsimd_h2v2_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v2_upsample_avx2(cinfo->max_v_samp_factor, cinfo->output_width,
input_data, output_data_ptr);
Expand All @@ -415,6 +433,9 @@ GLOBAL(void)
jsimd_h2v1_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v1_upsample_avx2(cinfo->max_v_samp_factor, cinfo->output_width,
input_data, output_data_ptr);
Expand Down Expand Up @@ -469,6 +490,9 @@ GLOBAL(void)
jsimd_h2v2_fancy_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v2_fancy_upsample_avx2(cinfo->max_v_samp_factor,
compptr->downsampled_width, input_data,
Expand All @@ -483,6 +507,9 @@ GLOBAL(void)
jsimd_h2v1_fancy_upsample(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JSAMPARRAY input_data, JSAMPARRAY *output_data_ptr)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_h2v1_fancy_upsample_avx2(cinfo->max_v_samp_factor,
compptr->downsampled_width, input_data,
Expand Down Expand Up @@ -542,6 +569,9 @@ jsimd_h2v2_merged_upsample(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
void (*avx2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);
void (*sse2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);

if (simd_support == ~0U)
init_simd();

switch (cinfo->out_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_h2v2_extrgb_merged_upsample_avx2;
Expand Down Expand Up @@ -590,6 +620,9 @@ jsimd_h2v1_merged_upsample(j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
void (*avx2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);
void (*sse2fct) (JDIMENSION, JSAMPIMAGE, JDIMENSION, JSAMPARRAY);

if (simd_support == ~0U)
init_simd();

switch (cinfo->out_color_space) {
case JCS_EXT_RGB:
avx2fct = jsimd_h2v1_extrgb_merged_upsample_avx2;
Expand Down Expand Up @@ -679,6 +712,9 @@ GLOBAL(void)
jsimd_convsamp(JSAMPARRAY sample_data, JDIMENSION start_col,
DCTELEM *workspace)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_convsamp_avx2(sample_data, start_col, workspace);
else
Expand Down Expand Up @@ -748,6 +784,9 @@ jsimd_can_fdct_float(void)
GLOBAL(void)
jsimd_fdct_islow(DCTELEM *data)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_fdct_islow_avx2(data);
else
Expand Down Expand Up @@ -809,6 +848,9 @@ jsimd_can_quantize_float(void)
GLOBAL(void)
jsimd_quantize(JCOEFPTR coef_block, DCTELEM *divisors, DCTELEM *workspace)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_quantize_avx2(coef_block, divisors, workspace);
else
Expand Down Expand Up @@ -963,6 +1005,9 @@ jsimd_idct_islow(j_decompress_ptr cinfo, jpeg_component_info *compptr,
JCOEFPTR coef_block, JSAMPARRAY output_buf,
JDIMENSION output_col)
{
if (simd_support == ~0U)
init_simd();

if (simd_support & JSIMD_AVX2)
jsimd_idct_islow_avx2(compptr->dct_table, coef_block, output_buf,
output_col);
Expand Down

0 comments on commit 4e028ec

Please sign in to comment.