Some build requirement updates

1. Make OpenMP a build requirement as non-OpenMP builds
   - don't help for anything
   - add code burden that would require special testing
   - don't have acceptable performance
   External code like rawspeed and libraw are not touched.

2. With current compilers we can assume proper vectorizing support so the
   DT_NO_VECTORIZATION and DT_NO_SIMD_HINTS can be removed.

3. As OpenMP and SSE2 are no build options, don't show them in the log file.

Author: jenshannoschwalm

CMakeLists.txt

@@ -75,21 +75,19 @@ else()
     set(RPATH_ORIGIN $ORIGIN)
 endif()
 
-if (USE_OPENMP)
-    if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR
-        CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
+if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR
+    CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
 
-        # Clang has an option to specify the OpenMP standard to use. Specify it.
-        # FIXME: Implement this in FindOpenMP.cmake
-        set(OPENMP_VERSION_SPECIFIER "-fopenmp-version=51")
+    # Clang has an option to specify the OpenMP standard to use. Specify it.
+    # FIXME: Implement this in FindOpenMP.cmake
+    set(OPENMP_VERSION_SPECIFIER "-fopenmp-version=51")
 
-        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OPENMP_VERSION_SPECIFIER}")
-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OPENMP_VERSION_SPECIFIER}")
-    endif()
-
-    find_package(OpenMP 4.5 REQUIRED)
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OPENMP_VERSION_SPECIFIER}")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OPENMP_VERSION_SPECIFIER}")
 endif()
 
+find_package(OpenMP 4.5 REQUIRED)
+
 # Check for base threading library
 set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
 find_package(Threads REQUIRED)

DefineOptions.cmake

@@ -6,7 +6,6 @@ option(DONT_USE_INTERNAL_LUA "Never fall back to the intree copy of lua" ON)
 option(USE_KWALLET "Build kwallet password storage back-end" ON)
 option(USE_LIBSECRET "Build libsecret password storage back-end" ON)
 option(USE_UNITY "Use libunity to report progress in the launcher" OFF)
-option(USE_OPENMP "Use OpenMP threading support." ON)
 option(USE_OPENCL "Use OpenCL support." ON)
 option(USE_GRAPHICSMAGICK "Use GraphicsMagick library for image import." ON)
 option(USE_IMAGEMAGICK "Use ImageMagick library for image import." OFF)

build.sh

@@ -35,7 +35,7 @@ CMAKE_OPTIONS_FROM_CMDLINE=""
 
 PRINT_HELP=0
 
-FEATURES="AI CAMERA COLORD GMIC GRAPHICSMAGICK IMAGEMAGICK JXL KWALLET LIBSECRET LUA MAC_INTEGRATION MAP OPENCL OPENEXR OPENMP UNITY WEBP"
+FEATURES="AI CAMERA COLORD GMIC GRAPHICSMAGICK IMAGEMAGICK JXL KWALLET LIBSECRET LUA MAC_INTEGRATION MAP OPENCL OPENEXR UNITY WEBP"
 
 # prepare a lowercase version with a space before and after
 # it's very important for parse_feature, has no impact in for loop expansions

src/CMakeLists.txt

@@ -273,25 +273,23 @@ list(APPEND LIBS "${CMAKE_THREAD_LIBS_INIT}")
 # Need to explicitly link against math library.
 list(APPEND LIBS "-lm")
 
-if(USE_OPENMP)
-  if(OpenMP_C_FLAGS)
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
-  endif()
+if(OpenMP_C_FLAGS)
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
+endif()
 
-  if(OpenMP_CXX_FLAGS)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
-  endif()
+if(OpenMP_CXX_FLAGS)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
+endif()
 
-  if(OpenMP_C_INCLUDE_DIRS)
-    include_directories(${OpenMP_C_INCLUDE_DIRS})
-  endif()
+if(OpenMP_C_INCLUDE_DIRS)
+  include_directories(${OpenMP_C_INCLUDE_DIRS})
+endif()
 
-  if(OpenMP_CXX_INCLUDE_DIRS)
-    include_directories(${OpenMP_CXX_INCLUDE_DIRS})
-  endif()
+if(OpenMP_CXX_INCLUDE_DIRS)
+  include_directories(${OpenMP_CXX_INCLUDE_DIRS})
+endif()
 
-  list(APPEND LIBS ${OpenMP_C_LIBRARIES} ${OpenMP_CXX_LIBRARIES})
-endif(USE_OPENMP)
+list(APPEND LIBS ${OpenMP_C_LIBRARIES} ${OpenMP_CXX_LIBRARIES})
 
 if(USE_DARKTABLE_PROFILING)
 	add_definitions(-DUSE_DARKTABLE_PROFILING)
@@ -1134,9 +1132,6 @@ set(CPACK_COMPONENTS_ALL DTApplication DTDebugSymbols DTDocuments)
 
 if(USE_LIBRAW AND NOT (DONT_USE_INTERNAL_LIBRAW AND libraw_FOUND))
   set(LIBRAW_PATH "${CMAKE_CURRENT_SOURCE_DIR}/external/LibRaw" CACHE STRING "Relative path to libraw directory (default=CMAKE_CURRENT_SOURCE_DIR)")
-  if(NOT USE_OPENMP)
-    set(ENABLE_OPENMP OFF CACHE BOOL "")
-  endif()
   set(ENABLE_LCMS OFF CACHE BOOL "")
   set(ENABLE_JASPER OFF CACHE BOOL "")
 

src/chart/main.c

@@ -1910,9 +1910,7 @@ int main(int argc, char *argv[])
   SetErrorMode(SEM_FAILCRITICALERRORS);
 #endif
 
-#ifdef _OPENMP
   omp_set_num_threads(omp_get_num_procs());
-#endif
 
   int res = 1;
   dt_lut_t *self = (dt_lut_t *)calloc(1, sizeof(dt_lut_t));

src/common/ai/restore_rgb.c

@@ -738,13 +738,7 @@ void dt_restore_apply_detail_recovery(const float *original_4ch,
     = dt_alloc_align_float(npix);
   if(!lum_residual) return;
 
-#ifdef _OPENMP
-#pragma omp parallel for simd default(none)           \
-  dt_omp_firstprivate(original_4ch, denoised_4ch,     \
-                      lum_residual, npix)             \
-  schedule(simd:static)                               \
-  aligned(original_4ch, denoised_4ch, lum_residual:64)
-#endif
+  DT_OMP_FOR_SIMD(aligned(original_4ch, denoised_4ch, lum_residual:64))
   for(size_t i = 0; i < npix; i++)
   {
     const size_t p = i * 4;
@@ -764,13 +758,7 @@ void dt_restore_apply_detail_recovery(const float *original_4ch,
   dwt_denoise(lum_residual, width, height,
               DWT_DETAIL_BANDS, noise);
 
-#ifdef _OPENMP
-#pragma omp parallel for simd default(none)       \
-  dt_omp_firstprivate(denoised_4ch, lum_residual, \
-                      npix, alpha)                \
-  schedule(simd:static)                           \
-  aligned(denoised_4ch, lum_residual:64)
-#endif
+  DT_OMP_FOR_SIMD(aligned(denoised_4ch, lum_residual:64))
   for(size_t i = 0; i < npix; i++)
   {
     const size_t p = i * 4;

src/common/darktable.c

@@ -119,9 +119,7 @@
 #include <sys/varargs.h>
 #endif
 
-#ifdef _OPENMP
 #include <omp.h>
-#endif
 
 #ifdef USE_LUA
 #include "lua/configuration.h"
@@ -826,18 +824,6 @@ char *version = g_strdup_printf(
                "  Debug                  -> DISABLED\n"
 #endif
 
-#if defined(__SSE2__) && defined(__SSE__)
-               "  SSE2 optimizations     -> ENABLED\n"
-#else
-               "  SSE2 optimizations     -> DISABLED\n"
-#endif
-
-#ifdef _OPENMP
-               "  OpenMP                 -> ENABLED\n"
-#else
-               "  OpenMP                 -> DISABLED\n"
-#endif
-
 #ifdef HAVE_OPENCL
                "  OpenCL                 -> ENABLED\n"
 #else
@@ -1270,12 +1256,10 @@ int dt_init(int argc,
           dt_print(DT_DEBUG_ALWAYS,
                    "[dt_init --threads] requested %d ompthreads restricted to %d",
                    desired, possible);
-#ifdef _OPENMP
         dt_print(DT_DEBUG_ALWAYS,
                  "[dt_init --threads] using %d threads of %d for openmp parallel sections %s",
                  darktable.num_openmp_threads, (int)dt_get_num_procs(),
                  omp_get_dynamic() ? "(dynamic)" : "(static)");
-#endif
         k++;
         argv[k-1] = NULL;
         argv[k] = NULL;
@@ -1540,10 +1524,8 @@ int dt_init(int argc,
     darktable.gui = (dt_gui_gtk_t *)calloc(1, sizeof(dt_gui_gtk_t));
   }
 
-#ifdef _OPENMP
   omp_set_num_threads(darktable.num_openmp_threads);
   omp_set_dynamic(FALSE);
-#endif
 
 #ifdef USE_LUA
   dt_lua_init_early(L);

src/common/darktable.h

@@ -78,8 +78,7 @@ typedef unsigned int u_int;
 #include <sys/sysctl.h>
 #endif
 
-#ifdef _OPENMP
-# include <omp.h>
+#include <omp.h>
 
 /* See https://redmine.darktable.org/issues/12568#note-14 */
 # ifdef HAVE_OMP_FIRSTPRIVATE_WITH_CONST
@@ -91,41 +90,30 @@ typedef unsigned int u_int;
 # endif/* HAVE_OMP_FIRSTPRIVATE_WITH_CONST */
 
 #ifndef dt_omp_sharedconst
-#ifdef _OPENMP
 #if defined(__clang__) || __GNUC__ > 8
-# define dt_omp_sharedconst(...) shared(__VA_ARGS__)
+  #define dt_omp_sharedconst(...) shared(__VA_ARGS__)
 #else
   // GCC 8.4 throws string of errors "'x' is predetermined 'shared' for 'shared'" if we explicitly declare
   //  'const' variables as shared
-# define dt_omp_sharedconst(var, ...)
+  #define dt_omp_sharedconst(var, ...)
 #endif
-#endif /* _OPENMP */
 #endif /* dt_omp_sharedconst */
 
 #ifndef dt_omp_nontemporal
 // Clang 10+ supports the nontemporal() OpenMP directive
 // GCC 9 recognizes it as valid, but does not do anything with it
 // GCC 10+ ???
 #if (__clang__+0 >= 10 || __GNUC__ >= 9)
-#  define dt_omp_nontemporal(...) nontemporal(__VA_ARGS__)
+  #define dt_omp_nontemporal(...) nontemporal(__VA_ARGS__)
 #else
 // GCC7/8 only support OpenMP 4.5, which does not have the nontemporal() directive.
-#  define dt_omp_nontemporal(var, ...)
+  #define dt_omp_nontemporal(var, ...)
 #endif
 #endif /* dt_omp_nontemporal */
 
 #define DT_OMP_STRINGIFY(...) #__VA_ARGS__
 #define DT_OMP_PRAGMA(...) _Pragma(DT_OMP_STRINGIFY(omp __VA_ARGS__))
 
-#else /* _OPENMP */
-
-# define omp_get_max_threads() 1
-# define omp_get_thread_num() 0
-
-#define DT_OMP_PRAGMA(...)
-
-#endif /* _OPENMP */
-
 #define DT_OMP_SIMD(clauses) DT_OMP_PRAGMA(simd clauses)
 #define DT_OMP_DECLARE_SIMD(clauses) DT_OMP_PRAGMA(declare simd clauses)
 #define DT_OMP_FOR(clauses) DT_OMP_PRAGMA(parallel for default(firstprivate) schedule(static) clauses)
@@ -778,29 +766,17 @@ gboolean dt_supported_image(const gchar *filename);
 
 static inline size_t dt_get_num_threads()
 {
-#ifdef _OPENMP
   return (size_t)CLAMP(omp_get_num_procs(), 1, darktable.num_openmp_threads);
-#else
-  return 1;
-#endif
 }
 
 static inline size_t dt_get_num_procs()
 {
-#ifdef _OPENMP
   return (size_t)MAX(1, omp_get_num_procs());
-#else
-  return 1;
-#endif
 }
 
 static inline int dt_get_thread_num()
 {
-#ifdef _OPENMP
   return omp_get_thread_num();
-#else
-  return 0;
-#endif
 }
 
 #define DT_INITHASH 5381
@@ -871,20 +847,6 @@ static inline float *dt_calloc_perthread_float(const size_t n,
 #define dt_get_bythread(buf, padsize, tnum) \
   DT_IS_ALIGNED((buf) + ((padsize) * (tnum)))
 
-// Most code in dt assumes that the compiler is capable of
-// auto-vectorization.  In some cases, this will yield suboptimal code
-// if the compiler in fact does NOT auto-vectorize.  Uncomment the
-// following line for such a compiler.
-
-//#define DT_NO_VECTORIZATION
-
-// For some combinations of compiler and architecture, the compiler
-// may actually emit inferior code if given a hint to vectorize a
-// loop.  Uncomment the following line if such a combination is the
-// compilation target.
-
-//#define DT_NO_SIMD_HINTS
-
 // copy the RGB channels of a pixel; includes the 'alpha' channel as
 // well if faster due to vectorization, but subsequent code should
 // ignore the value of the alpha unless explicitly set afterwards

src/common/densecrf.cc

@@ -21,9 +21,7 @@
 
 #include <exception>
 #include <math.h>
-#ifdef _OPENMP
 #include <omp.h>
-#endif
 
 #include "common/darktable.h"
 #include "common/densecrf.h"

src/common/dttypes.h

@@ -62,14 +62,7 @@ typedef DT_ALIGNED_PIXEL float dt_aligned_pixel_t[4];
 // a 3x3 matrix, padded to permit SSE instructions to be used for multiplication and addition
 typedef float DT_ALIGNED_ARRAY dt_colormatrix_t[4][4];
 
-// To be able to vectorize per-pixel loops, we need to operate on all four channels, but if the compiler does
-// not auto-vectorize, doing so increases computation by 1/3 for a channel which typically is ignored anyway.
-// Select the appropriate number of channels over which to loop to produce the fastest code.
-#ifdef DT_NO_VECTORIZATION
-#define DT_PIXEL_SIMD_CHANNELS 3
-#else
 #define DT_PIXEL_SIMD_CHANNELS 4
-#endif
 
 // A function to compute how many pixels each thread should process in a parallelized for loop.
 // For very small RoIs on a CPU with lots of threads, the last one or two hardware threads can end
@@ -91,7 +84,7 @@ static inline size_t dt_cacheline_chunks(const size_t npixels, const size_t nthr
 //         {
 //           src[k] = dest[k] / 3.0f;
 //         }
-#if defined(_OPENMP) && defined(OPENMP_SIMD_) && !defined(DT_NO_SIMD_HINTS)
+#if defined(OPENMP_SIMD_)
 //https://stackoverflow.com/questions/45762357/how-to-concatenate-strings-in-the-arguments-of-pragma
 #define _DT_Pragma_(x) _Pragma(#x)
 #define _DT_Pragma(x) _DT_Pragma_(x)