mpimd-csc / flexiblas

FlexiBLAS - A BLAS and LAPACK wrapper library with runtime exchangeable backends. This is only a mirror of https://gitlab.mpi-magdeburg.mpg.de/software/flexiblas-release
https://www.mpi-magdeburg.mpg.de/projects/flexiblas
GNU Lesser General Public License v3.0
36 stars 7 forks source link

Switch to explicit prototypes #33

Closed Enchufa2 closed 1 year ago

Enchufa2 commented 1 year ago

-Wstrict-prototypes is starting to be a thing. See e.g. this, and we are already talking about modernizing C code in Fedora. I also started to see warnings on CRAN.

Enchufa2 commented 1 year ago

Just wanted to note that there are more warnings from the -Wpedantic flag here. These are all pointer conversions such as

flexiblas_api_standalone.c:67:11: warning: initializing 'void *' with an expression of type 'int (*)(void)' converts between void pointer and function pointer [-Wpedantic]
    void *ptr_self = &flexiblas_avail;
          ^          ~~~~~~~~~~~~~~~~
flexiblas_api_standalone.c:73:15: warning: assigning to 'int (*)()' from 'void *' converts between void pointer and function pointer [-Wpedantic]
        fnptr = ptr_next;
              ^ ~~~~~~~~
grisuthedragon commented 1 year ago

The strict prototypes issue is ongoing. Regarding the pendantic setting, It could be that we cannot fix it.

grisuthedragon commented 1 year ago

Some update on this.... Pedantic is possible for C but not for Fortran, since there we import stuff from the reference implementation, which does not fulfill these strict requirement as well.

Enchufa2 commented 1 year ago

Thanks for looking into this too. In the case CRAN asks about fixing them, this is a very reasonable point for a waiver.

grisuthedragon commented 1 year ago

I will take a look at the reference LAPACK sources and open an issue there. Before the pedantic problem is not fixed on their side, it is a bad idea to fix it in FlexiBLAS.

The pedantic warnings are fixed in the upcoming release with LAPACK 3.11.

Enchufa2 commented 1 year ago

I will take a look at the reference LAPACK sources and open an issue there.

Thanks!

Before the pedantic problem is not fixed on their side, it is a bad idea to fix it in FlexiBLAS.

Agree.

The pedantic warnings are fixed in the upcoming release with LAPACK 3.11.

Great!

Enchufa2 commented 1 year ago

Is v3.3.0 ready? I've seen that you pushed to GitLab.

grisuthedragon commented 1 year ago

@Enchufa2 Its ready now, the sync to github failed.

Enchufa2 commented 1 year ago

Note that there are still some non-explicit prototypes. In particular,

  flexiblas_api_standalone.c:62:20: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:88:31: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:490:30: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:516:45: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:521:29: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:526:44: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:531:24: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:536:39: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:541:25: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:546:40: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:551:22: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:556:37: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
  flexiblas_api_standalone.c:562:45: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
grisuthedragon commented 1 year ago

Can you provide the full compiler flags and the gcc version? Since I didn't them in my setup.

Enchufa2 commented 1 year ago

This is on CRAN. Apart from -Wstrict-prototypes, the other flags don't matter. Here's one of the offending lines:

https://github.com/mpimd-csc/flexiblas/blob/24f41fdc0330dd57521cb311fd1c1b53eb5220bc/src/flexiblas_api_standalone.c#L62

Clearly it needs a void between the parentheses. It's the same case for all the other line numbers.

grisuthedragon commented 1 year ago

That's weired. The line 62 isn't a prototype. It is the implementation and therefore not required. The corresponding prototypes are in flexiblas_api.h which is generated during the cmake procedure. There, the functions are defined like:

extern int flexiblas_avail(void);

I build it on U20.04 with gcc 9.4.0 and the following commandline, and I do not get any warning:

/usr/bin/cc -DFLEXIBLAS_CBLAS -DFLEXIBLAS_LAPACK -D_FILE_OFFSET_BITS=64 -D_NONE_ \
  -D_POSIX_C_SOURCE=200809L -Dflexiblas_api_EXPORTS \
 -I/home/dev/work/software/flexiblas/src -I/home/dev/work/software/flexiblas/build/include \ 
 -I/home/dev/work/software/flexiblas/include -I/home/dev/work/software/flexiblas/build \
 -I/home/dev/work/software/flexiblas/libcscutils/include -I/home/dev/work/software/flexiblas/build/libcscutils/include \
 -I/home/dev/work/software/flexiblas/libcscutils/src -I/home/dev/work/software/flexiblas/build/libcscutils/src \
 -I/home/dev/work/software/flexiblas/build/libcscutils/include/cscutils  -fPIC -std=c99 -fstack-protector-strong \
-fstack-clash-protection -D_FILE_OFFSET_BITS=64 -O3 -DNDEBUG -O3 -Wpedantic -Wstrict-prototypes \ 
-Wcast-qual -fPIC   -Wpedantic -o CMakeFiles/flexiblas_api.dir/flexiblas_api_standalone.c.o  \
 -c /home/dev/work/software/flexiblas/src/flexiblas_api_standalone.c
Enchufa2 commented 1 year ago

The new warnings were upon submission. I found the specific command and output digging in the email (see here). I copy the output here because that file will eventually be garbage-collected:

* installing *source* package ‘flexiblas’ ...
** using staged installation
** libs
using C compiler: ‘Debian clang version 15.0.6’
clang-15  -I"/home/hornik/tmp/R-d-clang-15/include" -DNDEBUG   -I/usr/local/include -DUSE_TYPE_CHECKING_STRICT -D_FORTIFY_SOURCE=2   -fpic  -g -O3 -Wall -pedantic -c flexiblas_api_standalone.c -o flexiblas_api_standalone.o
flexiblas_api_standalone.c:62:20: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
int flexiblas_avail()
                   ^
                    void
flexiblas_api_standalone.c:88:31: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
int flexiblas_get_color_output() {
                              ^
                               void
flexiblas_api_standalone.c:490:30: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
int flexiblas_get_num_threads()
                             ^
                              void
flexiblas_api_standalone.c:516:45: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
FLEXIBLAS_API_INT flexiblas_get_num_threads_()
                                            ^
                                             void
flexiblas_api_standalone.c:521:29: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
int openblas_get_num_threads()
                            ^
                             void
flexiblas_api_standalone.c:526:44: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
FLEXIBLAS_API_INT openblas_get_num_threads_()
                                           ^
                                            void
flexiblas_api_standalone.c:531:24: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
int mkl_get_num_threads()
                       ^
                        void
flexiblas_api_standalone.c:536:39: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
FLEXIBLAS_API_INT mkl_get_num_threads_()
                                      ^
                                       void
flexiblas_api_standalone.c:541:25: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
int blas_get_num_threads()
                        ^
                         void
flexiblas_api_standalone.c:546:40: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
FLEXIBLAS_API_INT blas_get_num_threads_()
                                       ^
                                        void
flexiblas_api_standalone.c:551:22: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
int acmlgetnumthreads()
                     ^
                      void
flexiblas_api_standalone.c:556:37: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
FLEXIBLAS_API_INT acmlgetnumthreads_()
                                    ^
                                     void
flexiblas_api_standalone.c:562:45: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
FLEXIBLAS_API_INT bli_thread_get_num_threads()
                                            ^
                                             void
13 warnings generated.
clang-15  -I"/home/hornik/tmp/R-d-clang-15/include" -DNDEBUG   -I/usr/local/include -DUSE_TYPE_CHECKING_STRICT -D_FORTIFY_SOURCE=2   -fpic  -g -O3 -Wall -pedantic -c init.c -o init.o
clang-15  -I"/home/hornik/tmp/R-d-clang-15/include" -DNDEBUG   -I/usr/local/include -DUSE_TYPE_CHECKING_STRICT -D_FORTIFY_SOURCE=2   -fpic  -g -O3 -Wall -pedantic -c wrapper.c -o wrapper.o
clang-15 -shared -L/home/hornik/tmp/R-d-clang-15/lib -Wl,-O1 -o flexiblas.so flexiblas_api_standalone.o init.o wrapper.o -L/home/hornik/tmp/R-d-clang-15/lib -lR
installing to /srv/hornik/tmp/CRAN/flexiblas.Rcheck/00LOCK-flexiblas/00new/flexiblas/libs
** R
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
** building package indices
** testing if installed package can be loaded from temporary location
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (flexiblas)

so clang 15 seems to be more strict?

grisuthedragon commented 1 year ago

I did a bunch of small experiments, and it turned out that clang 15 is the first clang version which is that strict. All version < 15 are not affected by this. I will fix it, and it will be in the next release.

Enchufa2 commented 1 year ago

Thanks!