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

Registration Reduction6x6Impl template datatype support. #3636

Merged
merged 3 commits into from
Jun 26, 2021

Conversation

reyanshsolis
Copy link
Collaborator

@reyanshsolis reyanshsolis commented Jun 24, 2021

Changes:
Adding template dtype support in Reduction6x6Impl.

Issue re-solved [by @stotko ]:
Adding

#if __CUDA_ARCH__ < 600
__device__ double atomicAdd(double *address, double val) {
    unsigned long long int *address_as_ull = (unsigned long long int *)address;
    unsigned long long int old = *address_as_ull, assumed;

    do {
        assumed = old;
        old = atomicCAS(
                address_as_ull, assumed,
                __double_as_longlong(val + __longlong_as_double(assumed)));

        // Note: uses integer comparison to avoid hang in case of NaN (since NaN
        // != NaN)
    } while (assumed != old);

    return __longlong_as_double(old);
}
#endif

throws error

.../Open3D/cpp/open3d/t/geometry/kernel/GeometryMacros.h(35): error: function "atomicAdd(double *, double)" has already been defined

Not adding the same, throws an error in some of the CIs, that atomicAdd(double *, double) is not defined.
[Refer to the CI failure in PR #3564 ]

D:\a\Open3D\Open3D\cpp\open3d/t/pipelines/kernel/Reduction6x6Impl.cuh(180): error : no instance of overloaded function "atomicAdd" matches the argument list [C:\Open3D\build\cpp\open3d\t\pipelines\kernel\tpipelines_kernel.vcxproj]
              argument types are: (double *, volatile double)
            detected during:
              instantiation of "void open3d::t::pipelines::kernel::ReduceSum6x6LinearSystem<T,BLOCK_SIZE>(int, __nv_bool, const T *, volatile T *, volatile T *, volatile T *, T *) [with T=double, BLOCK_SIZE=256ULL]" 

This change is Reviewable

@update-docs
Copy link

update-docs bot commented Jun 24, 2021

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@reyanshsolis reyanshsolis requested a review from stotko June 24, 2021 09:52
@reyanshsolis reyanshsolis changed the title atomicAdd double issue. Registration Reduction6x6Impl template datatype support. Jun 25, 2021
@reyanshsolis
Copy link
Collaborator Author

Thanks, @stotko for the help. I have changed the topic of this pr for template datatype support in the Reduction6x6Impl code, which is used later in my other PR for adding F64 support in registration. Could you please review for the same ?

@reyanshsolis reyanshsolis marked this pull request as ready for review June 25, 2021 13:55
Copy link
Contributor

@stotko stotko left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @reyanshsolis)


cpp/open3d/t/pipelines/kernel/Reduction6x6Impl.cuh, line 168 at r2 (raw file):

        if (tid == 0) {
            OPEN3D_ATOMIC_ADD(&global_sum[i + 0], local_sum0[0]);

nit: It's fine to keep the old non-portable atomicAdd calls here (and below) since the function will not work on __host__ at the moment.

@reyanshsolis
Copy link
Collaborator Author

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @reyanshsolis)

cpp/open3d/t/pipelines/kernel/Reduction6x6Impl.cuh, line 168 at r2 (raw file):

        if (tid == 0) {
            OPEN3D_ATOMIC_ADD(&global_sum[i + 0], local_sum0[0]);

nit: It's fine to keep the old non-portable atomicAdd calls here (and below) since the function will not work on __host__ at the moment.

Done.

@reyanshsolis reyanshsolis merged commit 09aa226 into master Jun 26, 2021
@reyanshsolis reyanshsolis deleted the rey/reductionF64 branch June 26, 2021 10:51
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.

2 participants