scalapack icon indicating copy to clipboard operation
scalapack copied to clipboard

Use `size_t` in `mr2d_malloc` instead of index-bound `Int`

Open gdonval opened this issue 1 year ago • 3 comments

In C, the type of the number of bytes passed to malloc is of type size_t.

The current code ties that number of bytes to the general "Index type" (Int) which is defined at compile time and does fail on 32-bits systems setting Int as 64-bits if the number exceeds the 32-bit range.

Additionally, this artificially limits 64-bit-element aux matrices to 2GB instead of 16GB with 32-bits signed indices. This is a limit I have reached.

Changing the type back to size_t fixes all of that.

gdonval avatar Oct 03 '23 12:10 gdonval

At least at home, all tests besides the two listed in issue #69 pass (make test). I hope these tests cover what is in REDIST.

gdonval avatar Oct 03 '23 16:10 gdonval

My NEGFLAG is wrong. I'll fix that tomorrow. Is there a special command to use to test the REDIST part of the xode or is it just not tested?

gdonval avatar Oct 03 '23 16:10 gdonval

@julielangou Can I kindly ask for you opinion on this as a maintainer?

The gist of it is that we have this signature mr2d_malloc(Int n), bounding the internal malloc to Int, which is the indexing type.

This PR operates this transformation: mr2d_malloc(Int n) -> mr2d_malloc(size_t n), yet ensures Int -> size_t conversion does not involve Int-negative values and ensures that no 64-bit values are passed to malloc on 32-bit systems.

The main advantage of this on 64-bit system is allowing the use of the full signed 32-bit indexing range instead of range / element size. I.e. unless have made a mistake the max AUX matrix size is now 16GB instead of 2GB previously.

gdonval avatar Oct 04 '23 11:10 gdonval