[Build] choose_qparams_tensor_out get wrong return type cause build fail on native Windows

[python3kgae]

🐛 Describe the bug

When building on native Windows, I encountered an undefined symbol error.

lld-link : error : undefined symbol: __declspec(dllimport) class std::tuple<class at::Tensor &, class at::Tensor &> __cdecl torch::executor::native::choose_qparams_tensor_out(class at::Tensor const &, __int64, __int64, double, enum c10::ScalarType, class at::Tensor &, class at::Tensor &)

This issue can be worked around with the following patch.

diff --git a/kernels/quantized/cpu/op_choose_qparams.cpp b/kernels/quantized/cpu/op_choose_qparams.cpp
index 47f261407..9bda17192 100644
--- a/kernels/quantized/cpu/op_choose_qparams.cpp
+++ b/kernels/quantized/cpu/op_choose_qparams.cpp
@@ -149,7 +149,7 @@ void choose_qparams(
 }
 } // namespace
 
-std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
+std::tuple<Tensor&, Tensor&> choose_qparams_tensor_out(
     const Tensor& input,
     int64_t quant_min,
     int64_t quant_max,
@@ -164,7 +164,7 @@ std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
   return {scale_out, zero_point_out};
 }
 
-::std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
+::std::tuple<Tensor&, Tensor&> choose_qparams_tensor_out(
     RuntimeContext& context,
     const Tensor& input,
     int64_t quant_min,

Versions

Collecting environment information...
PyTorch version: 2.5.0.dev20240716+cpu
Is debug build: False
CUDA used to build PyTorch: Could not collect
ROCM used to build PyTorch: N/A

OS: Microsoft Windows 11 Pro
GCC version: Could not collect
Clang version: 18.1.8
CMake version: version 3.30.2
Libc version: N/A

Python version: 3.10.0 | packaged by conda-forge | (default, Nov 10 2021, 13:20:59) [MSC v.1916 64 bit (AMD64)] (64-bit runtime)
Python platform: Windows-10-10.0.22631-SP0
Is CUDA available: False
CUDA runtime version: 12.2.140
CUDA_MODULE_LOADING set to: N/A
GPU models and configuration: GPU 0: NVIDIA GeForce RTX 3070 Ti
Nvidia driver version: 551.76
cuDNN version: Could not collect
HIP runtime version: N/A
MIOpen runtime version: N/A
Is XNNPACK available: True

CPU:
Architecture=9
CurrentClockSpeed=3501
DeviceID=CPU0
Family=107
L2CacheSize=16384
L2CacheSpeed=
Manufacturer=AuthenticAMD
MaxClockSpeed=3501
Name=AMD Ryzen Threadripper PRO 3975WX 32-Cores
ProcessorType=3
Revision=12544

Versions of relevant libraries:
[pip3] executorch==0.4.0a0+a70d070
[pip3] numpy==1.21.3
[pip3] torch==2.5.0.dev20240716+cpu
[pip3] torchaudio==2.4.0.dev20240716+cpu
[pip3] torchsr==1.0.4
[pip3] torchvision==0.20.0.dev20240716+cpu
[conda] executorch 0.4.0a0+a70d070 pypi_0 pypi
[conda] numpy 1.21.3 pypi_0 pypi
[conda] torch 2.5.0.dev20240716+cpu pypi_0 pypi
[conda] torchaudio 2.4.0.dev20240716+cpu pypi_0 pypi
[conda] torchsr 1.0.4 pypi_0 pypi
[conda] torchvision 0.20.0.dev20240716+cpu pypi_0 pypi

[kirklandsign]

Do we know which file is requiring the definition of std::tuple<class at::Tensor &, class at::Tensor &>? That's a strange behavior.

The workaround seems to cause dangling reference right?

[python3kgae]

The user of std::tuple<class at::Tensor &, class at::Tensor &> is in

executorch\pip-out\temp.win-amd64-cpython-310\cmake-out\kernels\quantized\quantized_ops_aot_lib\RegisterCPUCustomOps.cpp

The header

executorch\pip-out\temp.win-amd64-cpython-310\cmake-out\kernels\quantized\quantized_ops_aot_lib\CustomOpsNativeFunctions.h

has

TORCH_API at::Tensor & quantized_add_out(const at::Tensor & a, double a_scale, int64_t a_zero_point, int64_t a_quant_min, int64_t a_quant_max, const at::Tensor & b, double b_scale, int64_t b_zero_point, int64_t b_quant_min, int64_t b_quant_max, double out_scale, int64_t out_zero_point, int64_t out_quant_min, int64_t out_quant_max, at::Tensor & out);

declared.

WSL also has the same declare in

executorch\pip-out\temp.linux-x86_64-cpython-310\cmake-out\kernels\quantized\quantized_ops_aot_lib\CustomOpsNativeFunctions.h

[python3kgae]

In current implementation of choose_qparams_tensor_out in

executorch\kernels\quantized\cpu\op_choose_qparams.cpp

The return value is '{scale_out, zero_point_out}', which is just use the input parameter 'Tensor& scale_out' and 'Tensor& zero_point_out'

It should not cause reference dangling.

[python3kgae]

Tried to build with choose_qparams_tensor_out removed from

executorch\kernels\quantized\cpu\op_choose_qparams.cpp

./install_requirements.sh still success in WSL.

Does it mean choose_qparams_tensor_out is linked from other places?

[kirklandsign]

I see. So it means that choose_qparams_tensor_out should always be returning ::std::tuple<at::Tensor &,at::Tensor &> ?

[python3kgae]

I see. So it means that choose_qparams_tensor_out should always be returning ::std::tuple<at::Tensor &,at::Tensor &> ?

From what I see on native Windows build,
choose_qparams_tensor_out is defined in executorch\kernels\quantized\cpu\op_choose_qparams.cpp and used in executorch\pip-out\temp.win-amd64-cpython-310\cmake-out\kernels\quantized\quantized_ops_aot_lib\RegisterCPUCustomOps.cpp.

But that cannot explain why on WSL build ./install_requirements.sh still success after removing choose_qparams_tensor_out define in executorch\kernels\quantized\cpu\op_choose_qparams.cpp. :(

Can you confirm that choose_qparams_tensor_out is expected to be defined in executorch\kernels\quantized\cpu\op_choose_qparams.cpp and used in executorch\pip-out\temp.win-amd64-cpython-310\cmake-out\kernels\quantized\quantized_ops_aot_lib\RegisterCPUCustomOps.cpp?

If that is the case, choose_qparams_tensor_out should always be returning ::std::tuple<at::Tensor &,at::Tensor &>.

[kirklandsign]

If that is the case, choose_qparams_tensor_out should always be returning ::std::tuple<at::Tensor &,at::Tensor &>.

Good point. @larryliu0820 says probably in linux they don’t care about function return type when looking for symbols but in windows it seems like they started to check return type. So it didn't cause issue in the past, but now we get error on windows build.

[kirklandsign]

Let's see if CI passes on #4704

[kirklandsign]

Looks promising! Would you like to make another PR for your patch since you wrote it?

diff --git a/kernels/quantized/cpu/op_choose_qparams.cpp b/kernels/quantized/cpu/op_choose_qparams.cpp
index 47f261407..9bda17192 100644
--- a/kernels/quantized/cpu/op_choose_qparams.cpp
+++ b/kernels/quantized/cpu/op_choose_qparams.cpp
@@ -149,7 +149,7 @@ void choose_qparams(
 }
 } // namespace
 
-std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
+std::tuple<Tensor&, Tensor&> choose_qparams_tensor_out(
     const Tensor& input,
     int64_t quant_min,
     int64_t quant_max,
@@ -164,7 +164,7 @@ std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
   return {scale_out, zero_point_out};
 }
 
-::std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
+::std::tuple<Tensor&, Tensor&> choose_qparams_tensor_out(
     RuntimeContext& context,
     const Tensor& input,
     int64_t quant_min,

[python3kgae]

Looks promising! Would you like to make another PR for your patch since you wrote it?

diff --git a/kernels/quantized/cpu/op_choose_qparams.cpp b/kernels/quantized/cpu/op_choose_qparams.cpp
index 47f261407..9bda17192 100644
--- a/kernels/quantized/cpu/op_choose_qparams.cpp
+++ b/kernels/quantized/cpu/op_choose_qparams.cpp
@@ -149,7 +149,7 @@ void choose_qparams(
 }
 } // namespace
 
-std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
+std::tuple<Tensor&, Tensor&> choose_qparams_tensor_out(
     const Tensor& input,
     int64_t quant_min,
     int64_t quant_max,
@@ -164,7 +164,7 @@ std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
   return {scale_out, zero_point_out};
 }
 
-::std::tuple<Tensor, Tensor> choose_qparams_tensor_out(
+::std::tuple<Tensor&, Tensor&> choose_qparams_tensor_out(
     RuntimeContext& context,
     const Tensor& input,
     int64_t quant_min,

Sure. I'll create the PR tonight.