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

MNT: suppress compiler warning from cupyx.cusolver #7714

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

leofang
Copy link
Member

@leofang leofang commented Jul 11, 2023

No description provided.

@asi1024 asi1024 added prio:medium cat:code-fix Code refactoring that do not change behavior labels Jul 12, 2023
@kmaehashi
Copy link
Member

/test mini

LGTM! Confirmed the following warning (in main) disappeared:

https://github.com/cupy/cupy/actions/runs/5529072408/jobs/10086636516#step:6:414

    gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -D_FORCE_INLINES=1 -DCUPY_NO_CUDA=1 -I/home/runner/work/cupy/cupy/cupy/_core/include/cupy/cub -I/home/runner/work/cupy/cupy/cupy/_core/include -I/opt/hostedtoolcache/Python/3.9.17/x64/include/python3.9 -c cupy_backends/cuda/libs/cudnn.cpp -o build/temp.linux-x86_64-3.9/cupy_backends/cuda/libs/cudnn.o
    cupyx/cusolver.cpp: In function ‘PyObject* __pyx_pw_5cupyx_8cusolver_21_geqrf_orgqr_batched(PyObject*, PyObject*, PyObject*)’:
    cupyx/cusolver.cpp:20661:39: warning: ‘__pyx_v_orgqr_bufferSize’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    20661 |   __pyx_t_7 = __pyx_v_orgqr_bufferSize(__pyx_v_handle, __pyx_v_m, __pyx_v_mc, __pyx_v_mn, __pyx_v_x_ptr, __pyx_v_m, __pyx_v_tau_ptr, 0); if (unlikely(__pyx_t_7 == ((int)-1) && PyErr_Occurred())) __PYX_ERR(0, 965, __pyx_L1_error)
          |               ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    cupyx/cusolver.cpp:19182:9: note: ‘__pyx_v_orgqr_bufferSize’ was declared here
    19182 |   int (*__pyx_v_orgqr_bufferSize)(intptr_t, int, int, int, size_t, int, size_t, int __pyx_skip_dispatch);
          |         ^~~~~~~~~~~~~~~~~~~~~~~~
    cupyx/cusolver.cpp:20720:39: warning: ‘__pyx_v_orgqr’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    20720 |         __pyx_v_status = __pyx_v_orgqr(__pyx_v_handle, __pyx_v_m, __pyx_v_mc, __pyx_v_mn, __pyx_v_x_ptr, __pyx_v_m, __pyx_v_tau_ptr, __pyx_v_w_ptr, __pyx_v_buffersize, __pyx_v_info_ptr, __pyx_v_batch_size, __pyx_v_orig_n);
          |                          ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    cupyx/cusolver.cpp:19181:38: note: ‘__pyx_v_orgqr’ was declared here
    19181 |   __pyx_t_5cupyx_8cusolver_orgqr_ptr __pyx_v_orgqr;
          |                                      ^~~~~~~~~~~~~

kmaehashi
kmaehashi previously approved these changes Jul 12, 2023
@kmaehashi
Copy link
Member

https://ci.preferred.jp/cupy.win.cuda120/137846/

NameError: name 'sorgqr_bufferSize' is not defined

Hmm, can't assign cimported function to object? 🤔

@leofang
Copy link
Member Author

leofang commented Aug 22, 2023

NameError: name 'sorgqr_bufferSize' is not defined

Hmm, can't assign cimported function to object? 🤔

Sorry, I did it in a rush and didn't check carefully, it's fixed now in commit 7337ee0.

It seems Cython has the ability to infer if a pure function pointer is being assigned, without the programmer to explicitly spelling out the type. So, the previous way cdef object sorgqr_bufferSize = None was actually wrong. By providing an else branch, Cython is smart enough to 1. generate the functor signature and 2. suppress the warning on uninitialized values.

@leofang
Copy link
Member Author

leofang commented Aug 22, 2023

/test mini

@kmaehashi
Copy link
Member

Test failure unrelated.

@kmaehashi kmaehashi merged commit 65f12b4 into cupy:main Aug 23, 2023
46 of 48 checks passed
@kmaehashi kmaehashi added the to-be-backported Pull-requests to be backported to stable branch label Aug 23, 2023
@kmaehashi kmaehashi added this to the v13.0.0rc1 milestone Aug 23, 2023
chainer-ci pushed a commit to chainer-ci/cupy that referenced this pull request Aug 23, 2023
MNT: suppress compiler warning from `cupyx.cusolver`
@leofang leofang deleted the remove_warning branch August 23, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:code-fix Code refactoring that do not change behavior prio:medium to-be-backported Pull-requests to be backported to stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants