Add block synchronisation after reset of CG-level counters

[wence-]

This is needed because if we go round loops again, we might read before things have been reset.

• Tentatively closes [BUG]: data race in static_multimap pair_retrieve #336.

[rapids-bot]

Pull requests from external contributors require approval from a NVIDIA organization member with write permissions or greater before CI can begin.

[wence-]

I am not sure all of these syncs are necessary, but have commented on my rationale for them.

[PointKernel]

@wence- Thanks for putting up the PR. Is this ready for review?

[wence-]

@wence- Thanks for putting up the PR. Is this ready for review?

I think so. I think I don't understand fully the semantics of some of the early returns in some of the device functions, but I think these bits are right

[PointKernel]

I take a closer look at the proposed changes and find they might not be necessary since CG vote functions imply synchronizations. @wence- Does the race check pass after adding those syncs?

[wence-]

I take a closer look at the proposed changes and find they might not be necessary since CG vote functions imply synchronizations. @wence- Does the race check pass after adding those syncs?

The modifications in kernels.cuh (to sync after the loop) are the ones necessary to pass race check. The modifications in device_view_impl.inl are not necessary for that test, but I think still necessary for correctness (I've responded to your comments to explain why I think they are necessary: basically, the warp-level vote functions don't provide memory barriers).

[PointKernel]

@wence- You are right. I was missing the point that warp vote functions don't communicate through memory and they don't guarantee any memory ordering.

To uncover more similar issues like this, I build your branch locally and run compute-sanitizer on multimap unit tests:

 compute-sanitizer --tool racecheck ./STATIC_MULTIMAP_TEST

The race check shows that there are 13 warnings with the current code. We can fix them all in this PR if you have the bandwidth. Otherwise, I will merge the PR as it is and put up follow-up PRs to solve the issue. I'm happy with either option.

[wence-]

@wence- You are right. I was missing the point that warp vote functions don't communicate through memory and they don't guarantee any memory ordering.

To uncover more similar issues like this, I build your branch locally and run compute-sanitizer on multimap unit tests:

 compute-sanitizer --tool racecheck ./STATIC_MULTIMAP_TEST

The race check shows that there are 13 warnings with the current code. We can fix them all in this PR if you have the bandwidth. Otherwise, I will merge the PR as it is and put up follow-up PRs to solve the issue. I'm happy with either option.

Let me have a go next week...

[wence-]

I have pushed fixes. STATIC_MULTIMAP_TEST is now racecheck clean with cuda 11.8 and 12.2, it is not clean with 12.0 which I think means either there are compute-sanitizer false positives or else bugs in 12.0.

[PointKernel]

LGTM. Thanks!