Eliminate literals passed to device_uvector::set_element_async #367
Conversation
harrism
added
3 - Ready for Review
| @@ -72,7 +72,8 @@ descend_quadtree(LengthsIter counts, | |||
| thrust::inclusive_scan( | |||
| rmm::exec_policy(stream), parent_counts, parent_counts + num_quads, parent_offsets.begin() + 1); | |||
|
|
|||
| parent_offsets.set_element_async(0, 0, stream); | |||
| uint32_t init{0}; | |||
| parent_offsets.set_element_async(0, init, stream); | |||
There was a problem hiding this comment.
Why eliminate one 0 literal and not the other?
There was a problem hiding this comment.
Great question. Only the value parameter (second parameter) is pass-by-reference, so that's the only one we are disallowing to be a literal. The first parameter, the index, is pass-by-value so it's perfectly safe to use a literal since it will be copied rather than referenced, and so we don't have to worry about the lifetime of the temporary inside set_element_async.
There was a problem hiding this comment.
That answers my question, yes. But why have one argument be pass by reference and one pass by value? They're both primitives. The 0 is only going to be used by the callee, and the init is going to be passed around and modified by others but has to be initialized here? Thanks!
After rapidsai/rmm#725 is merged, this PR updates cuspatial to eliminate passing literal values to device_uvector::set_element_async. Companion PR to rapidsai/cuspatial#367 Authors: - Mark Harris (@harrism) Approvers: - Seunghwa Kang (@seunghwak) - Alex Fender (@afender) - Andrei Schaffer (@aschaffer) URL: #1453
harrism
requested review from
trxcllnt and
cwharris
and removed request for
zhangjianting
cwharris
added
4 - Waiting on Author
|
@gpucibot merge |
After rapidsai/rmm#725 is merged, this PR updates cuspatial to eliminate passing literal values to
device_uvector::set_element_async.