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

kmp_settings.cpp fails to compile on Windows with error C2131: expression did not evaluate to a constant #48263

Closed
zmodem opened this issue Jan 28, 2021 · 5 comments
Labels
bugzilla Issues migrated from bugzilla openmp

Comments

@zmodem
Copy link
Collaborator

zmodem commented Jan 28, 2021

Bugzilla Link 48919
Resolution FIXED
Resolved on Jan 29, 2021 22:22
Version unspecified
OS Windows NT
Blocks #48246
CC @AndreyChurbanov,@tstellar
Fixed by commit(s) 7f5ad0e 074ad6d

Extended Description

in a VS 2019 x86 Native Tools Command Prompt

with llvmorg-12.0.0-rc1 checked out

cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_PROJECTS="clang;openmp" ..\llvm
ninja omp

C:\src\llvm.monorepo\openmp\runtime\src\kmp_settings.cpp(3358): error C2131: expression did not evaluate to a constant
C:\src\llvm.monorepo\openmp\runtime\src\kmp_settings.cpp(3358): note: failure was caused by a read of a variable outside its lifetime
C:\src\llvm.monorepo\openmp\runtime\src\kmp_settings.cpp(3358): note: see usage of 'ntraits'

Bisection points to

commit 927af4b
Author: Nawrin Sultana nawrin.sultana@intel.com
Date: Mon Nov 2 16:17:37 2020 -0600

[OpenMP] Modify OMP_ALLOCATOR environment variable

This patch sets the def-allocator-var ICV based on the environment variables
provided in OMP_ALLOCATOR. Previously, only allowed value for OMP_ALLOCATOR
was a predefined memory allocator. OpenMP 5.1 specification allows predefined
memory allocator, predefined mem space, or predefined mem space with traits in
OMP_ALLOCATOR. If an allocator can not be created using the provided environment
variables, the def-allocator-var is set to omp_default_mem_alloc.

Differential Revision: https://reviews.llvm.org/D94985

openmp/runtime/src/kmp_settings.cpp | 391 ++++++++++++++++++++++++------
openmp/runtime/test/env/omp51_alloc_env.c | 31 +++
2 files changed, 353 insertions(+), 69 deletions(-)
create mode 100644 openmp/runtime/test/env/omp51_alloc_env.c

As of this morning the same error happens on the main branch too.

@zmodem
Copy link
Collaborator Author

zmodem commented Jan 28, 2021

C:\src\llvm.monorepo\openmp\runtime\src\kmp_settings.cpp(3358): error C2131:
expression did not evaluate to a constant
C:\src\llvm.monorepo\openmp\runtime\src\kmp_settings.cpp(3358): note:
failure was caused by a read of a variable outside its lifetime
C:\src\llvm.monorepo\openmp\runtime\src\kmp_settings.cpp(3358): note: see
usage of 'ntraits'

I think what MSVC might be trying to say in this convoluted way, is that it doesn't like

omp_alloctrait_t traits[ntraits]

because ntraits is not constant, and so this would be a variable-length array which isn't support in C++ (though often allowed as an extension).

The fix would be to not use a VLA here.

@AndreyChurbanov
Copy link
Collaborator

I've posted the fix https://reviews.llvm.org/D95627 for review.

@AndreyChurbanov
Copy link
Collaborator

Fixed by commit 7f5ad0e.

@zmodem
Copy link
Collaborator Author

zmodem commented Jan 29, 2021

Fixed by commit
https://github.com/llvm/llvm-project/commit/
7f5ad0e.

Thanks! That seems to work on my machine.

Tom, can we merge this to the release branch?

@tstellar
Copy link
Collaborator

Merged: 074ad6d

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla openmp
Projects
None yet
Development

No branches or pull requests

3 participants