From 17eaf9b053238b3efec303e9c94008201ca42462 Mon Sep 17 00:00:00 2001 From: Changming Sun Date: Wed, 13 Dec 2023 11:11:13 -0800 Subject: [PATCH] Fix a build warning in SparseTensor code for 32-bit build configs (#18766) ### Description The warning is: ``` C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(88,54): warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.1812949Z with 2023-12-08T20:58:48.2144272Z [ 2023-12-08T20:58:48.2145285Z Derived=Eigen::Map,0,Eigen::Stride<0,0>> 2023-12-08T20:58:48.2801935Z ] 2023-12-08T20:58:48.2804047Z C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(82,8): message : while compiling class template member function 'void onnxruntime::contrib::`anonymous-namespace'::SparseToDenseCsr::operator ()(const onnxruntime::contrib::`anonymous-namespace'::ComputeCtx &,const onnxruntime::SparseTensor &,const onnxruntime::Tensor &,onnxruntime::Tensor &) const' [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.2806197Z C:\a\_work\1\s\include\onnxruntime\core/framework/data_types_internal.h(302,27): message : see the first reference to 'onnxruntime::contrib::`anonymous-namespace'::SparseToDenseCsr::operator ()' in 'onnxruntime::utils::mltype_dispatcher_internal::CallableDispatchableHelper::Invoke' (compiling source file C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc) [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.2871783Z C:\a\_work\1\s\include\onnxruntime\core/framework/data_types_internal.h(438,100): message : see reference to class template instantiation 'onnxruntime::contrib::`anonymous-namespace'::SparseToDenseCsr' being compiled (compiling source file C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc) [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.2893010Z C:\a\_work\1\s\include\onnxruntime\core/framework/data_types_internal.h(414,5): message : see reference to function template instantiation 'void onnxruntime::utils::MLTypeCallDispatcher::InvokeWithLeadingTemplateArgs,onnxruntime::contrib::`anonymous-namespace'::ComputeCtx&,const T&,const onnxruntime::Tensor&,onnxruntime::Tensor&>(onnxruntime::contrib::`anonymous-namespace'::ComputeCtx &,const T &,const onnxruntime::Tensor &,onnxruntime::Tensor &) const' being compiled [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.2894476Z with 2023-12-08T20:58:48.2911521Z [ 2023-12-08T20:58:48.2912457Z Fn=onnxruntime::contrib::`anonymous-namespace'::SparseToDenseCsr, 2023-12-08T20:58:48.3067840Z T=onnxruntime::SparseTensor 2023-12-08T20:58:48.3068863Z ] (compiling source file C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc) 2023-12-08T20:58:48.3195854Z C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(198,11): message : see reference to function template instantiation 'void onnxruntime::utils::MLTypeCallDispatcher::Invoke(onnxruntime::contrib::`anonymous-namespace'::ComputeCtx &,const T &,const onnxruntime::Tensor &,onnxruntime::Tensor &) const' being compiled [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.3197946Z with 2023-12-08T20:58:48.3198565Z [ 2023-12-08T20:58:48.3199093Z T=onnxruntime::SparseTensor 2023-12-08T20:58:48.3905678Z ] 2023-12-08T20:58:48.3907275Z C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(198,36): message : see the first reference to 'onnxruntime::utils::MLTypeCallDispatcher::Invoke' in 'onnxruntime::contrib::SparseToDenseMatMul::Compute' [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.3910999Z ##[warning]onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(88,43): Warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data 2023-12-08T20:58:48.3912734Z 182>C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(88,43): warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.3913414Z with 2023-12-08T20:58:48.3913660Z [ 2023-12-08T20:58:48.3914001Z Derived=Eigen::Map,0,Eigen::Stride<0,0>> 2023-12-08T20:58:48.3914499Z ] 2023-12-08T20:58:48.3914743Z qlinear_concat.cc 2023-12-08T20:58:48.3917082Z ##[warning]onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(92,74): Warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data 2023-12-08T20:58:48.3918624Z 182>C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(92,74): warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.5534583Z with 2023-12-08T20:58:48.5541266Z [ 2023-12-08T20:58:48.5542401Z Derived=Eigen::Map,0,Eigen::Stride<0,0>> 2023-12-08T20:58:48.5544914Z ] 2023-12-08T20:58:48.5548670Z ##[warning]onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(92,63): Warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data 2023-12-08T20:58:48.5552099Z 182>C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(92,63): warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.5553712Z with 2023-12-08T20:58:48.5555569Z [ 2023-12-08T20:58:48.5556779Z Derived=Eigen::Map,0,Eigen::Stride<0,0>> 2023-12-08T20:58:48.5558707Z ] 2023-12-08T20:58:48.5561428Z ##[warning]onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(93,90): Warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data 2023-12-08T20:58:48.5565624Z 182>C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(93,90): warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.5566354Z with 2023-12-08T20:58:48.5568185Z [ 2023-12-08T20:58:48.5569305Z Derived=Eigen::Map,0,Eigen::Stride<0,0>> 2023-12-08T20:58:48.5571339Z ] 2023-12-08T20:58:48.5574864Z ##[warning]onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(93,77): Warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data 2023-12-08T20:58:48.5577866Z 182>C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(93,77): warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.5578562Z with 2023-12-08T20:58:48.5580399Z [ 2023-12-08T20:58:48.5581503Z Derived=Eigen::Map,0,Eigen::Stride<0,0>> 2023-12-08T20:58:48.5583465Z ] 2023-12-08T20:58:48.5587661Z ##[warning]onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(88,54): Warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data 2023-12-08T20:58:48.5590705Z 182>C:\a\_work\1\s\onnxruntime\contrib_ops\cpu\math\sparse_dense_matmul.cc(88,54): warning C4244: 'argument': conversion from 'const __int64' to 'Eigen::EigenBase::Index', possible loss of data [C:\a\_work\1\b\RelWithDebInfo\onnxruntime_providers.vcxproj] 2023-12-08T20:58:48.5591396Z with 2023-12-08T20:58:48.5593220Z [ 2023-12-08T20:58:48.5593693Z Derived=Eigen::Map,0,Eigen::Stride<0,0>> 2023-12-08T20:58:48.5595955Z ] ``` And the warning in #18195 ### Motivation and Context AB#22894 --------- Co-authored-by: Dmitri Smirnov --- .../cpu/math/sparse_dense_matmul.cc | 71 ++++++++++++------- onnxruntime/core/util/math_cpuonly.h | 2 +- .../contrib_ops/math/matmul_sparse_test.cc | 2 - .../azure-pipelines/linux-ci-pipeline.yml | 3 +- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/onnxruntime/contrib_ops/cpu/math/sparse_dense_matmul.cc b/onnxruntime/contrib_ops/cpu/math/sparse_dense_matmul.cc index b00b10ad64..46a8b70d28 100644 --- a/onnxruntime/contrib_ops/cpu/math/sparse_dense_matmul.cc +++ b/onnxruntime/contrib_ops/cpu/math/sparse_dense_matmul.cc @@ -47,7 +47,6 @@ struct ComputeCtx { float alpha; }; -#if !defined(__i386__) && !defined(_M_IX86) && !defined(__wasm__) && !defined(__ANDROID__) template inline void SparseDenseMatMulImpl(const ComputeCtx& ctx, const ConstSparseMatrixMap& map_A, const ConstEigenMatrixMapRowMajor& map_B, EigenMatrixMapRowMajor& output_map) { @@ -64,7 +63,8 @@ inline void SparseDenseMatMulImpl(const ComputeCtx& ctx, const ConstSparseMatrix template <> inline void SparseDenseMatMulImpl(const ComputeCtx& ctx, const ConstSparseMatrixMap& map_A, - const ConstEigenMatrixMapRowMajor& map_B, EigenMatrixMapRowMajor& output_map) { + const ConstEigenMatrixMapRowMajor& map_B, + EigenMatrixMapRowMajor& output_map) { if (ctx.trans_A && ctx.trans_B) { output_map = map_A.transpose() * ctx.alpha * map_B.transpose(); } else if (ctx.trans_A && !ctx.trans_B) { @@ -84,21 +84,47 @@ struct SparseToDenseCsr { const auto& b_dims = B.Shape().GetDims(); const auto& out_dims = output.Shape().GetDims(); auto csr_view = A.AsCsr(); + const Eigen::Index* inner_index_pointer = nullptr; + const Eigen::Index* outer_index_pointer = nullptr; + // For auto-release the above two pointers when they are not NULL. + std::unique_ptr buffer_holder_inner, buffer_holder_outer; + if constexpr (std::is_integral::value && + std::is_signed::value && + (sizeof(Eigen::Index) == sizeof(int64_t))) { + // On macOS the following reinterpret_cast is necessary because Eigen::Index is an alias of `long` but int64_t is + // `long long`. Though they have the same size, compilers still do not allow an implicit casting between them. + inner_index_pointer = reinterpret_cast(csr_view.Inner().Data()); + outer_index_pointer = reinterpret_cast(csr_view.Outer().Data()); + } else { + // In a 32-bit build we need to cast the following two tensors to 32 bits + gsl::span inner_data = csr_view.Inner().DataAsSpan(); + gsl::span outer_data = csr_view.Outer().DataAsSpan(); + buffer_holder_inner.reset(new Eigen::Index[inner_data.size()]); + buffer_holder_outer.reset(new Eigen::Index[outer_data.size()]); + inner_index_pointer = buffer_holder_inner.get(); + outer_index_pointer = buffer_holder_outer.get(); - ConstSparseMatrixMap map_A(a_dims[0], a_dims[1], A.NumValues(), - csr_view.Outer().Data(), - csr_view.Inner().Data(), + std::transform(inner_data.begin(), inner_data.end(), + buffer_holder_inner.get(), [](int64_t v) -> Eigen::Index { + return narrow(v); + }); + std::transform(outer_data.begin(), outer_data.end(), + buffer_holder_outer.get(), [](int64_t v) -> Eigen::Index { + return narrow(v); + }); + } + ConstSparseMatrixMap map_A(narrow(a_dims[0]), narrow(a_dims[1]), + narrow(A.NumValues()), outer_index_pointer, inner_index_pointer, A.Values().Data()); - ConstEigenMatrixMapRowMajor map_B(B.Data(), b_dims[0], b_dims[1]); - EigenMatrixMapRowMajor output_map(output.MutableData(), out_dims[0], out_dims[1]); + ConstEigenMatrixMapRowMajor map_B(B.Data(), narrow(b_dims[0]), narrow(b_dims[1])); + EigenMatrixMapRowMajor output_map(output.MutableData(), narrow(out_dims[0]), + narrow(out_dims[1])); // XXX: Consider re-writing it as a parallel loop as Eigen requires it to use OpenMP // XXX: Consider vectorization SparseDenseMatMulImpl(ctx, map_A, map_B, output_map); } }; -#endif //! defined(__i386__) && !defined(_M_IX86) && !defined(__wasm__) && !defined(__ANDROID__) - template inline T Mul(T a_value, float, T b_value) { return a_value * b_value; @@ -121,9 +147,11 @@ struct SparseToDenseCoo { auto coo_view = A.AsCoo(); const auto& ind_dims = coo_view.Indices().Shape().GetDims(); ORT_RETURN_IF_NOT(ind_dims.size() == 2, "COO indices must be 2-D, got: ", ind_dims.size()); - ConstEigenMatrixMapRowMajor a_indicies_map(coo_view.Indices().Data(), narrow(ind_dims[0]), narrow(ind_dims[1])); + ConstEigenMatrixMapRowMajor a_indicies_map(coo_view.Indices().Data(), narrow(ind_dims[0]), + narrow(ind_dims[1])); ConstEigenMatrixMapRowMajor map_b(B.Data(), narrow(b_dims[0]), narrow(b_dims[1])); - EigenMatrixMapRowMajor output_map(output.MutableData(), narrow(out_dims[0]), narrow(out_dims[1])); + EigenMatrixMapRowMajor output_map(output.MutableData(), narrow(out_dims[0]), + narrow(out_dims[1])); output_map.setZero(); const auto rhs_right = (ctx.trans_B) ? b_dims[0] : b_dims[1]; @@ -140,7 +168,8 @@ struct SparseToDenseCoo { ORT_RETURN_IF_NOT(m < out_left, "COO m index: ", m, " is out of bounds of out_left: ", out_left); const T a_value = a_values[i]; for (int64_t n = 0; n < rhs_right; ++n) { - const T b_value = (ctx.trans_B) ? map_b(narrow(n), narrow(k)) : map_b(narrow(k), narrow(n)); + const T b_value = + (ctx.trans_B) ? map_b(narrow(n), narrow(k)) : map_b(narrow(k), narrow(n)); output_map(narrow(m), narrow(n)) += Mul(a_value, ctx.alpha, b_value); } } @@ -170,8 +199,9 @@ Status SparseToDenseMatMul::Compute(OpKernelContext* ctx) const { const auto inner_B = (trans_b_attr_) ? b_dims[1] : b_dims[0]; const auto outer_B = (trans_b_attr_) ? b_dims[0] : b_dims[1]; - ORT_RETURN_IF_NOT(inner_A == inner_B, "Can not multiply A and B as inner dimension does not match. inner_A: ", - inner_A, " vs inner_B: ", inner_B); + ORT_RETURN_IF_NOT(inner_A == inner_B, + "Can not multiply A and B as inner dimension does not match. inner_A: ", inner_A, + " vs inner_B: ", inner_B); TensorShape output_shape{outer_A, outer_B}; auto* output = ctx->Output(0, output_shape); @@ -184,12 +214,10 @@ Status SparseToDenseMatMul::Compute(OpKernelContext* ctx) const { auto coo_view = A->AsCoo(); const auto num_dims = coo_view.Indices().Shape().NumDimensions(); ORT_RETURN_IF_NOT(num_dims == 2, "Expecting COO 2-D indices shape"); - ORT_RETURN_IF_NOT(A->Values().Shape().Size() * 2 == coo_view.Indices().Shape().Size(), "Expecting 2xValues == indices"); + ORT_RETURN_IF_NOT(A->Values().Shape().Size() * 2 == coo_view.Indices().Shape().Size(), + "Expecting 2xValues == indices"); auto status = t_disp.InvokeRet(compute_ctx, *A, *B, *output); ORT_RETURN_IF_ERROR(status); -// Eigen has a bug in x86 where it calculates reallocation size as -1 -// and throws bad_alloc -#if !defined(__i386__) && !defined(_M_IX86) && !defined(__wasm__) && !defined(__ANDROID__) } else if (A->Format() == SparseFormat::kCsrc) { auto csr_view = A->AsCsr(); ORT_RETURN_IF_NOT(A->Values().Shape().Size() == csr_view.Inner().Shape().Size(), @@ -199,11 +227,6 @@ Status SparseToDenseMatMul::Compute(OpKernelContext* ctx) const { } else { return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_ARGUMENT, "Currently support only COO and CSR(x64) formats"); } -#else - } else { - return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_ARGUMENT, "WASM and 32-bit builds support only COO format"); - } -#endif //! defined(__i386__) && !defined(_M_IX86) && !defined(__wasm__) && !defined(__ANDROID__) return Status::OK(); } @@ -211,4 +234,4 @@ Status SparseToDenseMatMul::Compute(OpKernelContext* ctx) const { } // namespace contrib } // namespace onnxruntime -#endif //! defined(DISABLE_SPARSE_TENSORS) \ No newline at end of file +#endif //! defined(DISABLE_SPARSE_TENSORS) diff --git a/onnxruntime/core/util/math_cpuonly.h b/onnxruntime/core/util/math_cpuonly.h index f4fa3aa54b..73caf9f861 100644 --- a/onnxruntime/core/util/math_cpuonly.h +++ b/onnxruntime/core/util/math_cpuonly.h @@ -93,7 +93,7 @@ template using ConstEigenMatrixMap = Eigen::Map>; template -using ConstSparseMatrixMap = Eigen::Map>; +using ConstSparseMatrixMap = Eigen::Map>; template using ConstEigenArrayMap = Eigen::Map>; diff --git a/onnxruntime/test/contrib_ops/math/matmul_sparse_test.cc b/onnxruntime/test/contrib_ops/math/matmul_sparse_test.cc index b77c5e0ed9..8f8946e0d4 100644 --- a/onnxruntime/test/contrib_ops/math/matmul_sparse_test.cc +++ b/onnxruntime/test/contrib_ops/math/matmul_sparse_test.cc @@ -140,7 +140,6 @@ void resize(Index size, double reserveSizeFactor = 0) { } */ #if !defined(DISABLE_SPARSE_TENSORS) -#if !defined(__i386__) && !defined(_M_IX86) && !defined(__wasm__) && !defined(__ANDROID__) TEST(SparseToDenseMatMul, TestCsr) { constexpr int64_t rows = 9; constexpr int64_t cols = 9; @@ -261,7 +260,6 @@ TEST(SparseToDenseMatMul, TestCsr) { tester.Run(OpTester::ExpectResult::kExpectSuccess); } } -#endif // //!defined(__i386__) && !defined(_M_IX86) && !defined(__wasm__) && !defined(__ANDROID__) TEST(SparseToDenseMatMul, TestCoo) { constexpr int64_t rows = 9; diff --git a/tools/ci_build/github/azure-pipelines/linux-ci-pipeline.yml b/tools/ci_build/github/azure-pipelines/linux-ci-pipeline.yml index f46febee17..64b78dca50 100644 --- a/tools/ci_build/github/azure-pipelines/linux-ci-pipeline.yml +++ b/tools/ci_build/github/azure-pipelines/linux-ci-pipeline.yml @@ -106,8 +106,7 @@ stages: ls $(Build.BinariesDirectory)/gccbin/bin mkdir $(Build.BinariesDirectory)/arm32build cd $(Build.BinariesDirectory)/arm32build - # TODO: fix the warnings and remove the --compile-no-warning-as-error arg - cmake --compile-no-warning-as-error $(Build.SourcesDirectory)/cmake -Donnxruntime_ENABLE_CPUINFO=OFF -DPython_EXECUTABLE=/usr/bin/python3 -DPYTHON_EXECUTABLE=/usr/bin/python3 -DCMAKE_BUILD_TYPE=Debug -DCMAKE_TOOLCHAIN_FILE=$(Build.SourcesDirectory)/cmake/linux_arm32_crosscompile_toolchain.cmake -G Ninja + cmake $(Build.SourcesDirectory)/cmake -Donnxruntime_ENABLE_CPUINFO=OFF -DPython_EXECUTABLE=/usr/bin/python3 -DPYTHON_EXECUTABLE=/usr/bin/python3 -DCMAKE_BUILD_TYPE=Debug -DCMAKE_TOOLCHAIN_FILE=$(Build.SourcesDirectory)/cmake/linux_arm32_crosscompile_toolchain.cmake -G Ninja ninja rm -rf $(Build.BinariesDirectory)/arm32build $(Build.BinariesDirectory)/gccbin displayName: Cross-compile for Linux ARM32 and ARM64