This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
Change size_t to int within for loop to fix windows build error #14740
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Are these the only changes needed to fix the windows build error? I thought there are more for loops with size_t as index in the code. |
| @@ -156,7 +156,7 @@ void SgMKLDNNFCOp::Forward(const OpContext &ctx, | |||
| int32_t *quantized_bias_ptr = cached_bias_.data().dptr<int32_t>(); | |||
| size_t bias_size = bias.shape().Size(); | |||
| #pragma omp parallel for num_threads(engine::OpenMP::Get()->GetRecommendedOMPThreadCount()) | |||
| for (size_t i = 0; i < bias_size; ++i) { | |||
| for (int i = 0; i < static_cast<int>(bias_size); ++i) { | |||
There was a problem hiding this comment.
Is the static_cat from size_t to int safety if the value of bias_size is large than the range of int?
There was a problem hiding this comment.
Already changed to index_t.
|
@apeforest That are the only two cases we found in local. There were other places reported in #14203 . I think some of them have already been fixed via other PRs. |
TaoLv
approved these changes
Apr 19, 2019
|
Thank you for the fix. @yinghu5 You may be interested. |
|
Thank you for the contribution @ciyongch. It's now merged. |
haohuanw
pushed a commit
to haohuanw/incubator-mxnet
that referenced
this pull request
Jun 23, 2019
…he#14740) * change size_t to int for supporting OpenMp 2.0 windows build * change int to index_t
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This patch is to fix the build error of using "size_t" within for loop on Windows Platform, since MSVC only support OpenMP 2.0 standard, which will cause the "error C3016: 'i' : index variable in OpenMP 'for' statement must have signed integral type".
@pengzhao-intel @TaoLv
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments