[Openmp-commits] [PATCH] D64534: Remove OMP spec versioning

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added inline comments.


================
Comment at: runtime/CMakeLists.txt:72-73
 libomp_check_variable(LIBOMP_LIB_TYPE normal profile stubs)
-set(LIBOMP_OMP_VERSION 50 CACHE STRING
-  "The OpenMP version (50/45/40/30)")
-libomp_check_variable(LIBOMP_OMP_VERSION 50 45 40 30)
 # Set the OpenMP Year and Month assiociated with version
-if(${LIBOMP_OMP_VERSION} GREATER 50 OR ${LIBOMP_OMP_VERSION} EQUAL 50)
-  set(LIBOMP_OMP_YEAR_MONTH 201611)
-elseif(${LIBOMP_OMP_VERSION} GREATER 45 OR ${LIBOMP_OMP_VERSION} EQUAL 45)
-  set(LIBOMP_OMP_YEAR_MONTH 201511)
-elseif(${LIBOMP_OMP_VERSION} GREATER 40 OR ${LIBOMP_OMP_VERSION} EQUAL 40)
-  set(LIBOMP_OMP_YEAR_MONTH 201307)
-elseif(${LIBOMP_OMP_VERSION} GREATER 30 OR ${LIBOMP_OMP_VERSION} EQUAL 30)
-  set(LIBOMP_OMP_YEAR_MONTH 201107)
-else()
-  set(LIBOMP_OMP_YEAR_MONTH 200505)
-endif()
+set(LIBOMP_OMP_YEAR_MONTH 201611)
 set(LIBOMP_MIC_ARCH knc CACHE STRING
----------------
AndreyChurbanov wrote:
> Hahnfeld wrote:
> > A quick `grep` shows only one usage in `omp_lib.{f,f90,h}.var` where we can actually hardcode the version.
> > 
> > (This would finally mean that the `.var` files only access constant variables like `LIBOMP_VERSION_*` and we could avoid replacing them at configure time. But this can be cleaned up in future patches...)
> Jonas,
> Could you clarify a bit why hardcoding values in 3 files can be better than having values in a single place?  We will need to change versions once support for new specification implemented, and doing this in one place looks preferable over changing constants in 3 files.
> 
> Or am I missing something?
> 
> Thanks, Andrey
My reasoning was that we don't need to replace variables in `include/` anymore because no value depends on the CMake configuration. I didn't think about raising the value once a  new spec is released, good point. I'd still argue that this is pretty rare (every 2-3 years based on the recent history), but I acknowledge that inconsistency between the different files is bad.

I'm fine with leaving the variable for now and possibly revisit the topic later on. Thanks!


Repository:
  rOMP OpenMP

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64534/new/

https://reviews.llvm.org/D64534