LTO mismatch with `dgemm` with RcppArmadillo and R core definitions

[mattfidler]

Hi @eddelbuettel

I think there may be a RcppArmadillo LTO issue that is described by the following log:

https://www.stats.ox.ac.uk/pub/bdr/LTO/RxODE.out

To save you time in parsing, it boils down to the following:

/data/gannet/ripley/R/R-devel/include/R_ext/BLAS.h:207:1: warning: type of ‘dgemm_’ does not match original declaration [-Wlto-type-mismatch]
  207 | F77_NAME(dgemm)(const char *transa, const char *transb, const int *m,
      | ^
/data/gannet/ripley/R/test-4.2/RcppArmadillo/include/armadillo_bits/def_blas.hpp:115:8: note: type mismatch in parameter 14
  115 |   void arma_fortran(arma_dgemm)(const char* transA, const char* transB, const blas_int* m, const blas_int* n, const blas_int* k, const double*   alpha, const double*   A, const blas_int* ldA, const double*   B, const blas_int* ldB, const double*   beta, double*   C, const blas_int* ldC, blas_len transA_len, blas_len transB_len) ARMA_NOEXCEPT;
      |        ^
/data/gannet/ripley/R/test-4.2/RcppArmadillo/include/armadillo_bits/def_blas.hpp:115:8: note: type ‘blas_len’ should match type ‘void’
/data/gannet/ripley/R/test-4.2/RcppArmadillo/include/armadillo_bits/def_blas.hpp:115:8: note: ‘dgemm_’ was previously declared here

My guess is the LTO compile options have changed and become more restrictive (ie flto=10). I did (with your help) also change the headers to strict R headers, though I think that is unrelated

Perhaps my reading of the error message is incorrect. I thought I would bring it to your attention.

While I do not wish to, I may have to move and modify the armadillo headers (because of the fix by 2021-09-02 CRAN deadline).

[eddelbuettel]

Hm, I have yet to turn on LTO on the Debian (and Ubuntu) side of things, i.e. I have not enabled it for R, or Rcpp, or RcppArmadillo. So maybe it is something triggered by Prof Ripley on his side.

That said, it may be reproducible, and @conradsnicta is generally very receptive to clear and concise examples. So could I ask you to maybe try to cook up a (non-R, standalone) example program that with/without LTO options shows the difference? I probably have dgemm callers for BLAS here somewhere though we probably want to tickle it via RcppArmadillo.

[eddelbuettel]

Took another look at your full report here (reference from the standard results page here) and its a delicate issue as R (and LTO) look back to the basic C linkage yet Armadillo has a different (stacked, via arma_fortran() calling arma_dgemm()) setup. I am sure that will come up again. Hm. I may need to consult with @conradsnicta and see what we can do here medium-term as LTO will surely become more common and standard...

[mattfidler]

For a quick fix you could inline the blas routines and rename them; I don't think it is an ideal solution, but it worked for some of the matrix exponential calls used in the RxODE ode solving

https://github.com/nlmixrdevelopment/RxODE/blob/346edfe7be78c8998eccbb51b89a36929de84488/src/matexp.f#L12

This is not ideal it means you cant choose the blas that you want to do calculations.

When reading through the C code myself, I noticed the number of arguments for dgemm may be different in R than in standard blas. I think they used this trick as a safety check to get past the problems passing strings between C and fortran that we saw in #254, but I'm unsure their motivation

[eddelbuettel]

Quick fixes sometimes bite your backside in the medium / longer-term. A minimal yet reproducible example (outside of R) would be a good first step in getting towards this.

[mattfidler]

Hm. I could likely do this inside R but I dont think I can outside R. The blas R and standard blas definitions are clashing. I will try a simple R example first.

[eddelbuettel]

An R-only example would still help greatly. I can then either try to work from that or discuss it with @conradsnicta .

[eddelbuettel]

The other thing to do (for at least, while we work through this) is that you could add a -fno-lto flag.

[mattfidler]

Well I found a work around for the package for now. Interestingly enough, this came because of a code refactoring that actually didn't directly involve armadillo (found from a bisection)

nlmixrdevelopment/RxODE@52294dc

Alas however, I am unsure how this came up. I believe there is some bug somewhere, but since I can't figure out how to create it in a small reproducible example, and I know how to fix it for now, feel free to close this bug (unless you want to try to figure it out)

[eddelbuettel]

Yeah happy to close if that is ok with you.

Sometime strange things happen just with header reordering and alike -- maybe something was shadowing something elsewhere. It does smell a little local to your repo which is why closing may be wisest. I still have (optional?) conversion to lto for the R installation as a TODO so maybe we get to revisit this...

And by all means let's reopen if there is something new to review.

[mattfidler]

Thanks Dirk

[mattfidler]

You are right @conradsnicta,

The one thing I don't understand is why it thinks it needs to be void. To me, that means that is somehow being mucked up somewhere.

That could definitely cause the LTO errors that were observed.

At the same time, it doesn't always occur and I'm unsure what is causing it here.