zgimbutas / mwrap

Matlab MEX gateway generator
Other
9 stars 8 forks source link

suggest shouldn't need to pass in -D_INT64_T to use int64_t #1

Closed ahbarnett closed 4 years ago

ahbarnett commented 4 years ago

Dear Zydrunas, I am using your new 0.33.9 with our finufft mex interface, but find (by looking in the generated .cpp) that unless _INT64_T is #defined, it can't compile:

/home/alex/numerics/finufft/matlab/finufft_plan_mex.cpp: In function ‘void mexStub5(int, mxArray, int, const mxArray)’: /home/alex/numerics/finufft/matlab/finufft_plan_mex.cpp:842:16: error: ‘mxWrapGetArray_int64t’ was not declared in this scope in2 = mxWrapGetArray_int64_t(prhs[2], &mw_errtxt); ^~~~~~

Our .mw is here: https://github.com/flatironinstitute/finufft/tree/guruInterface_v2/matlab

If I put -D_INT64_T on mex compile line, it works. However, int64_t is a standard type, so shouldn't this be included by default?

The whole reason I'm using int64_t is because long is ambiguous; a compilation of such types on different platforms is here:

https://stackoverflow.com/questions/13604137/definition-of-int64-t#:~:text=int64_t%20is%20guaranteed%20by%20the,so%20it%20could%20be%20more.&text=Thus%2C%20int8_t%20denotes%20a%20signed,width%20of%20exactly%208%20bits.

Best, Alex

zgimbutas commented 4 years ago

Hi Alex,

We have had an issue with portability of int64_t type and friends between C and C++ compilers. Together with Libin we decided to include support for int64_t only if _INT64_T is defined, so the code is behaving as it should. int64_t is only in the new standards of C (starting from C99) and C++, and does not work well with older C. Including support for int64_t by default does break old compilers. The main question is whether there is a standard/portable way to detect int64_t support inside the compiler. I am not aware of one. Defining _INT64_T is a workaround.

Or should we drop pre-C99 compilers? This will include all gcc compilers before version 4.5.0 or so. I am still using gcc 4.4.7 as a windows cross-compiler but I can switch. Do you still use older gcc?

Best, Zydrunas

P.S. In my head: rap a.k.a. Weird Al Yankovich: In a 64-bit world you are a 32-bit user, you’ve got a new subreddit /r/total-loser

On Jun 10, 2020, at 8:44 AM, Alex Barnett notifications@github.com wrote:

Dear Zydrunas, I am using your new 0.33.9 with our finufft mex interface, but find (by looking in the generated .cpp) that unless _INT64_T is #defined, it can't compile:

/home/alex/numerics/finufft/matlab/finufft_plan_mex.cpp: In function ‘void mexStub5(int, mxArray, int, const mxArray)’: /home/alex/numerics/finufft/matlab/finufft_plan_mex.cpp:842:16: error: ‘mxWrapGetArray_int64t’ was not declared in this scope in2 = mxWrapGetArray_int64_t(prhs[2], &mw_errtxt); ^~~~~~

Our .mw is here: https://github.com/flatironinstitute/finufft/tree/guruInterface_v2/matlab https://github.com/flatironinstitute/finufft/tree/guruInterface_v2/matlab If I put -D_INT64_T on mex compile line, it works. However, int64_t is a standard type, so shouldn't this be included by default?

The whole reason I'm using int64_t is because long is ambiguous; a compilation of such types on different platforms is here:

https://stackoverflow.com/questions/13604137/definition-of-int64-t#:~:text=int64_t%20is%20guaranteed%20by%20the,so%20it%20could%20be%20more.&text=Thus%2C%20int8_t%20denotes%20a%20signed,width%20of%20exactly%208%20bits https://stackoverflow.com/questions/13604137/definition-of-int64-t#:~:text=int64_t%20is%20guaranteed%20by%20the,so%20it%20could%20be%20more.&text=Thus%2C%20int8_t%20denotes%20a%20signed,width%20of%20exactly%208%20bits.

Best, Alex

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zgimbutas/mwrap/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJ3GQNYCZTIS44TVH5M7TRV6L4JANCNFSM4N2OOVJA.

zgimbutas commented 4 years ago

I will try to add automatic detection if int64_t is used by mwrap, and generate corresponding copiers only if needed. This may be more user friendly and backward compatible.

On Jun 10, 2020, at 9:19 AM, Zydrunas Gimbutas zydrunas.gimbutas@gmail.com wrote:

Hi Alex,

We have had an issue with portability of int64_t type and friends between C and C++ compilers. Together with Libin we decided to include support for int64_t only if _INT64_T is defined, so the code is behaving as it should. int64_t is only in the new standards of C (starting from C99) and C++, and does not work well with older C. Including support for int64_t by default does break old compilers. The main question is whether there is a standard/portable way to detect int64_t support inside the compiler. I am not aware of one. Defining _INT64_T is a workaround.

Or should we drop pre-C99 compilers? This will include all gcc compilers before version 4.5.0 or so. I am still using gcc 4.4.7 as a windows cross-compiler but I can switch. Do you still use older gcc?

Best, Zydrunas

P.S. In my head: rap a.k.a. Weird Al Yankovich: In a 64-bit world you are a 32-bit user, you’ve got a new subreddit /r/total-loser

On Jun 10, 2020, at 8:44 AM, Alex Barnett <notifications@github.com mailto:notifications@github.com> wrote:

Dear Zydrunas, I am using your new 0.33.9 with our finufft mex interface, but find (by looking in the generated .cpp) that unless _INT64_T is #defined, it can't compile:

/home/alex/numerics/finufft/matlab/finufft_plan_mex.cpp: In function ‘void mexStub5(int, mxArray, int, const mxArray)’: /home/alex/numerics/finufft/matlab/finufft_plan_mex.cpp:842:16: error: ‘mxWrapGetArray_int64t’ was not declared in this scope in2 = mxWrapGetArray_int64_t(prhs[2], &mw_errtxt); ^~~~~~

Our .mw is here: https://github.com/flatironinstitute/finufft/tree/guruInterface_v2/matlab https://github.com/flatironinstitute/finufft/tree/guruInterface_v2/matlab If I put -D_INT64_T on mex compile line, it works. However, int64_t is a standard type, so shouldn't this be included by default?

The whole reason I'm using int64_t is because long is ambiguous; a compilation of such types on different platforms is here:

https://stackoverflow.com/questions/13604137/definition-of-int64-t#:~:text=int64_t%20is%20guaranteed%20by%20the,so%20it%20could%20be%20more.&text=Thus%2C%20int8_t%20denotes%20a%20signed,width%20of%20exactly%208%20bits https://stackoverflow.com/questions/13604137/definition-of-int64-t#:~:text=int64_t%20is%20guaranteed%20by%20the,so%20it%20could%20be%20more.&text=Thus%2C%20int8_t%20denotes%20a%20signed,width%20of%20exactly%208%20bits.

Best, Alex

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zgimbutas/mwrap/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJ3GQNYCZTIS44TVH5M7TRV6L4JANCNFSM4N2OOVJA.

ahbarnett commented 4 years ago

Ha ha! (We love Weird Al... because we're tackyyyyy.... :)

This makes total sense - but maybe it's time to update and regenerate the mwrap.pdf docs, and the README, with the such new features, if you are going to be the maintainer of mwrap (which I think you are!) The range of GCC used is ridiculous, eg our Flatiron cluster has centOS, default gcc 3.8.5 or something. Cheers, Alex

zgimbutas commented 4 years ago

Ok, let’s try more more time. I included automatic detection of int32_t, int64_t, uint32_t, and uint64_t types. The corresponding copiers are emitted only if these types are used in mwrap definitions which should make old scripts and compilers happy. I also switched -i8 flag to use int64_t and uint64_t types, so this flag should be easier to understand. The changes are in the master branch. I will delete r2018a branch since it is getting a bit stale.

On Jun 10, 2020, at 10:14 AM, Alex Barnett notifications@github.com wrote:

Ha ha! (We love Weird Al... because we're tackyyyyy.... :)

This makes total sense - but maybe it's time to update and regenerate the mwrap.pdf docs, and the README, with the such new features, if you are going to be the maintainer of mwrap (which I think you are!) The range of GCC used is ridiculous, eg our Flatiron cluster has centOS, default gcc 3.8.5 or something. Cheers, Alex

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/zgimbutas/mwrap/issues/1#issuecomment-642112955, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJ3GUAING66MV566JGU2DRV6WORANCNFSM4N2OOVJA.

ahbarnett commented 4 years ago

Works great for me on GCC 7.5.0 on i7, R2017a. Thanks! Alex