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

Enabling Globals for Unstructured Backends #1039

Merged
merged 5 commits into from
Oct 16, 2020

Conversation

mroethlin
Copy link
Contributor

Technical Description

This PR enables globals for the unstructured backends. Furthermore, an unreported issue where globals were not propagated from the wrapper class to the stencil class was fixed. Additionally, a bug in the unstructured cuda codegen was fixed when translating stencils that only use dense dimensions.

Resolves / Enhances

Fixes #1030
Fixes #1028

Notes

The methods to set and get globals in the cuda backend are on the inner stencils. This will be addressed in this issue. Also, a method to communicate globals from FORTRAN will need to be devised (not addressed yet).

Testing

New tests in dawn4py and a new unstructured integration test to test the correct operation of the CXXNaiveIco backend. CudaIco backend tested manually.

Dependencies

This PR is independent.

@mroethlin mroethlin requested a review from Stagno October 13, 2020 06:50
@mroethlin
Copy link
Contributor Author

launch jenkins

1 similar comment
@mroethlin
Copy link
Contributor Author

launch jenkins

@mroethlin
Copy link
Contributor Author

launch jenkins

Copy link
Contributor

@Stagno Stagno left a comment

Choose a reason for hiding this comment

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

There is also another problem, if you gtclang this (-backend=c++-naive)

#include "gtclang_dsl_defs/gtclang_dsl.hpp"
using namespace gtclang::dsl;

globals {
  double myval;
};

stencil Test {
  storage field_a, field_b;

  Do {
  	myval = 1.0;
  	vertical_region(k_start, k_end) {
	    field_a = field_b + myval;
    }
  	myval = 2.0;
    vertical_region(k_start, k_end) {
	    field_b = field_a + myval;
    }
  }
};

you'll see that setting a global via "control flow" will bypass the setter call and directly update m_globals of the wrapper class. I think the only "safe" option is having a const globals& in the inner class (in each backend) referencing a single globals object present in the wrapper class.

dawn/src/dawn/CodeGen/CodeGen.cpp Outdated Show resolved Hide resolved
dawn/src/dawn/CodeGen/CodeGen.cpp Outdated Show resolved Hide resolved
dawn/src/dawn/CodeGen/Cuda-ico/CudaIcoCodeGen.cpp Outdated Show resolved Hide resolved
dawn/src/dawn/CodeGen/Cuda-ico/CudaIcoCodeGen.cpp Outdated Show resolved Hide resolved
@mroethlin
Copy link
Contributor Author

launch jenkins

1 similar comment
@mroethlin
Copy link
Contributor Author

launch jenkins

@mroethlin
Copy link
Contributor Author

launch jenkins

@mroethlin mroethlin merged commit 713801a into MeteoSwiss-APN:master Oct 16, 2020
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.

Add support for local variables in cuda-ico Globals in CXXNaiveIcoCodeGen
2 participants