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

Deprecate ExecutionSpace::fence() as static function and make it non-static #2140

Closed
crtrott opened this issue May 14, 2019 · 8 comments
Closed
Assignees
Labels
Enhancement Improve existing capability; will potentially require voting

Comments

@crtrott
Copy link
Member

crtrott commented May 14, 2019

As part of the revamp to supporting execution space instances we need fences which are specific to the instance. So we deprecate the execution space specific fence functions as static and have them being non-static.

// Deprecated
Cuda::fence()
// Now
Cuda().fence()

Kokkos::fence() is still available as a static function.

As an internal implementation in order to fence all instances of a type we will have

Cuda::impl_static_fence()

Note we may change the function name for that impl thing later, or move it elsewhere.

@crtrott crtrott added the Enhancement Improve existing capability; will potentially require voting label May 14, 2019
@crtrott
Copy link
Member Author

crtrott commented May 14, 2019

Note if you have something like

traits::execution_space::fence()

You need to add typename

typename traits::execution_space().fence();

@crtrott
Copy link
Member Author

crtrott commented May 14, 2019

Sed lines:

find * -type f -name "*.hpp" | xargs sed -i 's|::fence()|().fence()|g'
find * -type f -name "*.hpp" | xargs sed -i 's|Kokkos().fence()|Kokkos::fence()|g'

Find all the ones needing a typename in front:

grep -R "::execution_space().fence()"

@mhoemmen
Copy link
Contributor

How long have Kokkos execution spaces had fence() as an instance method? I'm thinking about how to manage the interface change in Trilinos etc.

@crtrott
Copy link
Member Author

crtrott commented May 14, 2019

they had it as a static function for forever. But since you can call the static function like an instance method you can do the change now.

@crtrott
Copy link
Member Author

crtrott commented May 14, 2019

Nathan will do this on the Kokkos Promotion branch of Trilinos btw.

@ndellingwood
Copy link
Contributor

@crtrott

Nathan will do this on the Kokkos Promotion branch of Trilinos btw.

Yes I'll work on this, probably in kokkos-kernels first then Trilinos.

@ndellingwood
Copy link
Contributor

Packages likely impacted (aside from Kokkos + KokkosKernels of course):
ifpack2, intrepid2, moertel, panzer, phalanx, sacado, shylu fastilu, shylu tacho (mostly deprecated code), stokhos, teuchos, tpetra, trilinoscouplings (examples)

ndellingwood added a commit to kokkos/kokkos-kernels that referenced this issue May 15, 2019
See kokkos/kokkos#2140
The fence as a static member function of execution spaces was
deprecated, made non-static.
ndellingwood added a commit to kokkos/kokkos-kernels that referenced this issue May 16, 2019
ndellingwood added a commit to trilinos/Trilinos that referenced this issue May 16, 2019
See kokkos/kokkos#2140
The fence as a static member function of execution spaces was
deprecated and made non-static.
Impacted packages:
ifpack2, intrepid2, moertel, panzer, phalanx, sacado, shylu (fastilu and
    tacho), stokhos, teuchos, tpetra, trilinoscouplings
@ndellingwood
Copy link
Contributor

Updated in kokkos-kernels and on kokkos-promotion branch of Trilinos.

cabejackson pushed a commit to trilinos/Moertel that referenced this issue Aug 16, 2023
See kokkos/kokkos#2140
The fence as a static member function of execution spaces was
deprecated and made non-static.
Impacted packages:
ifpack2, intrepid2, moertel, panzer, phalanx, sacado, shylu (fastilu and
    tacho), stokhos, teuchos, tpetra, trilinoscouplings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

3 participants