The Wayback Machine - https://web.archive.org/web/20201001235009/https://github.com/rapidsai/cudf/issues/6241
Skip to content

/ cudf

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

Jump to bottom

[FEA] Improve readability of thread id based branching #6241

Open
devavret opened this issue Sep 15, 2020 · 5 comments
Open

[FEA] Improve readability of thread id based branching #6241

devavret opened this issue Sep 15, 2020 · 5 comments
Labels
cuIO feature request good first issue tech debt

Comments

@devavret
Copy link
Contributor

@devavret devavret commented Sep 15, 2020 •
edited

Improve readability of thread id based branches by giving them more descriptive names.

e.g.

if (!t) // is actually a t == 0

and

cudf/cpp/src/io/statistics/column_stats.cu

Line 285 in 57ef769

if (!(t & 0x1f)) {

Is actually a lane_id == 0
As demonstrated in #6241 (comment), prefer cooperative groups for this.

and

cudf/cpp/src/io/parquet/page_enc.cu

Line 1256 in 85cd56d

if ((t & 0x1f) == 0x1f) { s->scratch_red[t >> 5] = pos; }

is actually t < 32 lane_id == 31. (I think this might be an oversight, ignore as it might be fixed in #6238 ).
@OlivierNV
Copy link
Contributor

@OlivierNV OlivierNV commented Sep 15, 2020

Nope: last one is if (lane_id == 31), for those that prefer the "lane id" terminology.
@devavret
Copy link
Contributor Author

@devavret devavret commented Sep 15, 2020

Nope: last one is if (lane_id == 31), for those that prefer the "lane id" terminology.

Thanks. Thus demonstrated that it wasn't so readable for me 😆
@vuule
Copy link
Collaborator

@vuule vuule commented Sep 16, 2020 •
edited

Also, isn't !(t & 0x1f) actually t % 32 == 0, not t == 0 ?
Edit: confirmed for modulo. Also confirmed that the compiler generates the same code.
@jrhemstad
Copy link
Contributor

@jrhemstad jrhemstad commented Sep 16, 2020

Also, isn't !(t & 0x1f) actually t % 32 == 0, not t == 0 ?
Edit: confirmed for modulo. Also confirmed that the compiler generates the same code.

https://godbolt.org/z/a9ercr

Note that using CG also generates the same code, but it actually gives you access to the special laneid register.
@devavret
Copy link
Contributor Author

@devavret devavret commented Sep 16, 2020

Also, isn't !(t & 0x1f) actually t % 32 == 0, not t == 0 ?

That's what I wrote. lane_id == 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuIO feature request good first issue tech debt
4 participants
@devavret @jrhemstad @vuule @OlivierNV
You can’t perform that action at this time.