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

compiler: Avoid generating collapse(1) #2129

Merged
merged 1 commit into from
May 17, 2023
Merged

Conversation

FabioLuporini
Copy link
Contributor

Reason: NVC (at least version <= 23.1 , possible future as well) has a compilation bug (as in, the NVC compiler throws an internal fatal error while attempting to compile perfectly legal C+OpenMP code) that sometimes crops up when the collapse(1) clause is used.

Since (from the OpenMP specification)

The collapse clause associates one or more loops with the directive on which it appears for the purpose of identifying the portion of the depth of the canonical loop nest to which to apply the semantics of the directive. The argument n specifies the number of loops of the associated loop nest to which to apply those semantics. On all directives on which the collapse clause may appear, the effect is as if a value of one was specified for n if the collapse clause is not specified.

We here work around the annoying NVC issue by simply not generating collapse(1).

Bug hunting credits to @deckerla

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented May 17, 2023

Codecov Report

Merging #2129 (58efbeb) into master (526dcb8) will increase coverage by 0.14%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##           master    #2129      +/-   ##
==========================================
+ Coverage   87.03%   87.17%   +0.14%     
==========================================
  Files         219      222       +3     
  Lines       38996    39383     +387     
  Branches     5105     5121      +16     
==========================================
+ Hits        33941    34334     +393     
+ Misses       4481     4478       -3     
+ Partials      574      571       -3     
Impacted Files Coverage Δ
tests/test_gpu_openacc.py 5.69% <ø> (ø)
devito/passes/iet/languages/openacc.py 61.46% <66.66%> (ø)
tests/test_dle.py 96.47% <92.30%> (+0.02%) ⬆️
devito/passes/iet/languages/openmp.py 88.23% <100.00%> (+0.11%) ⬆️

... and 6 files with indirect coverage changes

Copy link
Contributor

@georgebisbas georgebisbas left a comment

Choose a reason for hiding this comment

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

I do not see any problem with the changes, though there are failing tests

@FabioLuporini FabioLuporini merged commit c7d15b6 into master May 17, 2023
@FabioLuporini FabioLuporini deleted the drop-collapse1-nvc branch May 17, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants