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

[Build] QNN EP build : implicit conversion loses integer precision #17778

Open
john-dance opened this issue Oct 3, 2023 · 2 comments
Open

[Build] QNN EP build : implicit conversion loses integer precision #17778

john-dance opened this issue Oct 3, 2023 · 2 comments
Assignees
Labels
build build issues; typically submitted using template ep:QNN issues related to QNN exeution provider

Comments

@john-dance
Copy link

john-dance commented Oct 3, 2023

Describe the issue

When building with use_qnn (clang, targeting Android), several files in the QNN Execution Provider sources use size_t and unit64_t interchangeably. Clang disagrees with that.

base_op_builder.cc - this one is a simple fix, just change line 212 to use auto

simple_op_builder.cc
qnn_backend_manager.cc

  • In these files, unt64_t is returned from some functions, but passed as size_t to other methods
simple_op_builder.cc:175:68: error: implicit conversion loses integer precision: 'int64_t' (aka 'long long') to 'size_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
    ParQuantizeLinearStd(&tensor_data.alpha, unpacked_data.data(), num_of_elements, scale, zero_point, thread_pool);

qnn_backend_manager.cc:361:87: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'size_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  std::unique_ptr<unsigned char[]> context_buffer = std::make_unique<unsigned char[]>(required_buffer_size);

qnn_backend_manager.cc:413:66: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'std::streamsize' (aka 'int') [-Werror,-Wshorten-64-to-32]
  of_stream.write(reinterpret_cast<char*>(context_buffer.get()), written_buffer_size);

qnn_backend_manager.cc:440:79: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'size_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  std::unique_ptr<unsigned char[]> buffer = std::make_unique<unsigned char[]>(buffer_size);

qnn_backend_manager.cc:444:84: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'std::streamsize' (aka 'int') [-Werror,-Wshorten-64-to-32]
  const auto& read_result = cache_file.read(reinterpret_cast<char*>(buffer.get()), buffer_size);


### Urgency

_No response_

### Target platform

Android

### Build script

 python3 tools/ci_build/build.py \
        --cmake_extra_defines "CMAKE_VERBOSE_MAKEFILE=ON" \
        --cmake_extra_defines "CMAKE_C_FLAGS=$extra_cflags" \
        --cmake_extra_defines "CMAKE_CXX_FLAGS=$extra_cflags" \
        --build_dir "$build_dir" \
        --config $build_type \
        --build_shared_lib \
        --parallel \
        --android \
        --android_abi $build_target \
        --android_api $VERSION \
        --android_sdk_path "$(get_android_home)" \
        --android_ndk_path "[...]" \
        --android_cpp_shared \
        --use_cache \
        --use_xnnpack \
        --use_nnapi \
        --use_qnn \
        --qnn_home="[...]" \
        --skip_tests \
        --path_to_protoc_exe "$PROTOC" \

### Error / output

(See above)

### Visual Studio Version

_No response_

### GCC / Compiler Version

_No response_
@john-dance john-dance added the build build issues; typically submitted using template label Oct 3, 2023
@github-actions github-actions bot added the platform:mobile issues related to ONNX Runtime mobile; typically submitted using template label Oct 3, 2023
@YUNQIUGUO YUNQIUGUO added ep:QNN issues related to QNN exeution provider and removed platform:mobile issues related to ONNX Runtime mobile; typically submitted using template labels Oct 3, 2023
@adrianlizarraga adrianlizarraga self-assigned this Oct 5, 2023
@adrianlizarraga
Copy link
Contributor

adrianlizarraga commented Oct 5, 2023

Thanks for reporting @john-dance. It looks like this an architecture with a 32-bit size_t. Can you please provide more details about the build target?

I also went ahead and created a draft PR that properly casts uint64/int64 to size_t in the locations you listed. Can you please try to compile with the PR's branch?

@john-dance
Copy link
Author

@adrianlizarraga - Thanks for reaching out.

The build target is armabi-v7a, so it is a 32-bit target.

Your draft handles most of the issues, but a few remain. As an aside, I previously didn't have an issue with conv.cc so either these were changes post 1.16.0 or changes in your diff.

src/onnxruntime/core/providers/xnnpack/nn/conv.cc:71:65: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_f32(op0_.get(), N, H, W, X.Data(), Y->MutableData(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:71:62: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_f32(op0_.get(), N, H, W, X.Data(), Y->MutableData(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:71:59: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_f32(op0_.get(), N, H, W, X.Data(), Y->MutableData(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:74:65: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qs8(op0_.get(), N, H, W, X.Data<int8_t>(), Y->MutableData<int8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:74:62: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qs8(op0_.get(), N, H, W, X.Data<int8_t>(), Y->MutableData<int8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:74:59: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qs8(op0_.get(), N, H, W, X.Data<int8_t>(), Y->MutableData<int8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:77:65: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qu8(op0_.get(), N, H, W, X.Data<uint8_t>(), Y->MutableData<uint8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:77:62: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qu8(op0_.get(), N, H, W, X.Data<uint8_t>(), Y->MutableData<uint8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:77:59: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qu8(op0_.get(), N, H, W, X.Data<uint8_t>(), Y->MutableData<uint8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:80:65: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qc8(op0_.get(), N, H, W, X.Data<int8_t>(), Y->MutableData<int8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:80:62: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qc8(op0_.get(), N, H, W, X.Data<int8_t>(), Y->MutableData<int8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
src/onnxruntime/core/providers/xnnpack/nn/conv.cc:80:59: warning: implicit conversion loses integer precision: 'const int64_t' (aka 'const long long') to 'size_t' (aka 'unsigned int') [-Wshorten-64-to-32]
status = xnn_setup_convolution2d_nhwc_qc8(op0_.get(), N, H, W, X.Data<int8_t>(), Y->MutableData<int8_t>(),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
12 warnings generated.
src/onnxruntime/core/providers/qnn/builder/opbuilder/base_op_builder.cc:213:28: error: implicit conversion loses integer precision: 'int64_t' (aka 'long long') to 'std::vector::value_type' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
permutations.push_back(p);
~~~~~~~~~ ^
src/onnxruntime/core/providers/qnn/builder/opbuilder/base_op_builder.cc:214:55: error: implicit conversion loses integer precision: 'int64_t' (aka 'long long') to 'size_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
new_tensor_shape_dims.push_back(tensor_shape_dims[p]);
~~~~~~~~~~~~~~~~~ ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build issues; typically submitted using template ep:QNN issues related to QNN exeution provider
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants