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

Make compiler_flags setting take precedence over the compiler flag env var #622

Conversation

MalachiTimothyPhillips
Copy link
Contributor

@MalachiTimothyPhillips MalachiTimothyPhillips commented Sep 19, 2022

Closes #621

@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #622 (01c1f6f) into main (2e576e6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #622   +/-   ##
=======================================
  Coverage   77.31%   77.32%           
=======================================
  Files         265      265           
  Lines       19592    19594    +2     
=======================================
+ Hits        15148    15151    +3     
+ Misses       4444     4443    -1     
Impacted Files Coverage Δ
src/occa/internal/modes/serial/device.cpp 90.27% <100.00%> (+0.54%) ⬆️
src/occa/internal/utils/sys.cpp 77.36% <0.00%> (+0.10%) ⬆️

Copy link
Member

@kris-rowe kris-rowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, the flags for the SYCL backend should also be updated also.

void setCompilerFlags(occa::json &dpcpp_properties) noexcept

(Currently Metal and OpenMP don't have their own env variables for compiler flags).

@MalachiTimothyPhillips
Copy link
Contributor Author

For consistency, the flags for the SYCL backend should also be updated also.

void setCompilerFlags(occa::json &dpcpp_properties) noexcept

(Currently Metal and OpenMP don't have their own env variables for compiler flags).

Done.
If we go this route, we should also do #623 .

@kris-rowe
Copy link
Member

@MalachiTimothyPhillips can you set your PR to merge into development.

@MalachiTimothyPhillips MalachiTimothyPhillips changed the base branch from main to development September 28, 2022 16:23
@MalachiTimothyPhillips
Copy link
Contributor Author

@MalachiTimothyPhillips can you set your PR to merge into development.

Done. Please let me know if you need anything else. Cheers!

@kris-rowe kris-rowe merged commit 50a8a9b into libocca:development Sep 28, 2022
kris-rowe pushed a commit that referenced this pull request Dec 16, 2022
…v var (#622)

* Make compiler_flags setting take precedence over the compiler flag env var

* Apply same change to SYCL backend.
kris-rowe pushed a commit that referenced this pull request Dec 19, 2022
…v var (#622)

* Make compiler_flags setting take precedence over the compiler flag env var

* Apply same change to SYCL backend.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

occa::json["compiler_flags"] should override environment variable
2 participants