-
Notifications
You must be signed in to change notification settings - Fork 4.3k
use size_t type for proper large tensor support #6201
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
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modernizes tensor dimension handling by changing key variables from int
to size_t
to properly support large tensors. This is important for avoiding integer overflow when dealing with large matrices and tensors where dimensions can exceed the range of 32-bit integers.
Key changes include:
- Converting stride variables from
int
tosize_t
in softmax implementations - Updating tensor step calculations to use
size_t
for memory offsets - Changing matrix dimension variables in GEMM operations to
size_t
Reviewed Changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/layer/x86/softmax_x86.cpp | Updates stride parameters to size_t in softmax functions |
src/layer/x86/gemm_x86.cpp | Changes tensor step calculations to size_t for GEMM operations |
src/layer/x86/gemm_int8.h | Updates matrix step variables to size_t in int8 GEMM functions |
src/layer/x86/convolution_packed_int8.h | Changes tensor step calculations to size_t in convolution |
src/layer/x86/convolution_packed.h | Updates tensor step variables to size_t |
src/layer/x86/convolution_im2col_gemm_int8.h | Changes offset calculations to size_t |
src/layer/x86/convolution_3x3_winograd_int8.h | Updates tensor step variables to size_t |
src/layer/x86/convolution_3x3_winograd.h | Changes tensor step calculations to size_t |
src/layer/slice.cpp | Updates size calculations to size_t |
src/layer/riscv/softmax_riscv.cpp | Changes stride parameters to size_t |
src/layer/riscv/gemm_riscv.cpp | Updates tensor step variables to size_t |
src/layer/glu.cpp | Changes offset calculation to size_t |
src/layer/gemm.cpp | Updates tensor step calculations to size_t |
src/layer/arm/* | Multiple ARM-specific files with similar size_t conversions |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6201 +/- ##
==========================================
- Coverage 96.17% 95.70% -0.47%
==========================================
Files 835 835
Lines 260827 265247 +4420
==========================================
+ Hits 250843 253860 +3017
- Misses 9984 11387 +1403 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
The binary size change of libncnn.so (bytes)
|
No description provided.