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

PTX: Default to 32-bit indexing of pointers. #444

Merged
merged 1 commit into from
May 11, 2023
Merged

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented May 9, 2023

This change sets the default index size for address calculations in PTX code to 32bit, in order to avoid extensions of 32-bit indices to 64-bit as part of instcombine when generating new GEP instructions (see JuliaLLVM/LLVM.jl#342 (comment)).

I'm not entirely sure this is correct, but it looks safe. It doesn't limit address calculations to 4GB, as you can still use 64-bit indices, but just defaults to 32-bit as these are generally more GPU friendly.
FWIW, the NVPTX back-end docs leave the index size undefined, and have it default to 64 bits.

x-ref JuliaGPU/CUDA.jl#1895

cc @vchuravy

@maleadt maleadt added the ptx Stuff about the NVIDIA PTX back-end. label May 9, 2023
@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -1.77 ⚠️

Comparison is base (5de5a5c) 87.06% compared to head (a514365) 85.30%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #444      +/-   ##
==========================================
- Coverage   87.06%   85.30%   -1.77%     
==========================================
  Files          24       24              
  Lines        2961     2994      +33     
==========================================
- Hits         2578     2554      -24     
- Misses        383      440      +57     
Impacted Files Coverage Δ
src/ptx.jl 93.80% <100.00%> (-0.03%) ⬇️

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@maleadt maleadt force-pushed the tb/ptx_dl_32bit branch from 0e79ee9 to a514365 Compare May 11, 2023 15:26
@maleadt
Copy link
Member Author

maleadt commented May 11, 2023

Let's try this out...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ptx Stuff about the NVIDIA PTX back-end.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant