#311 with macros

[sjaeckel]

Alternative approach to #311

NB: I'm not 100% sure if the last commit generate tommath_class from pre-processed code is a good idea resp. if the results are even correct!

[minad]

NB: I'm not 100% sure if the last commit generate tommath_class from pre-processed code is a good idea resp. if the results are even correct!

I also thought about that before. Maybe let's discuss this in a separate PR, also with respect to #301

[minad]

Looks good 👍

[czurnieden]

czurnieden ~/GITHUB/libtommath O (remove-bn_conversion)$ git log --pretty=format:"%an%x09" | grep Rube
czurnieden ~/GITHUB/libtommath O (remove-bn_conversion)$ 

Sorry, couldn't resist ;-)

But serious:
The old macros had to handle the different sizes of the standard C-types known only at compile-time and the easiest, if not the only way to do it was by a macro.
The types have fixed sizes now and, with the exception of the constants like MP_DIGIT_BIT all are known at coding-time. Wait, I see that you changed that, never mind.

There are five macros for 12 functions of which 6 are essentially one2one, I doubt the ROI is very high in this one ;-)
So: why only 32 and 64 bit? The macros are general enough with your changes—they should go public and allow the user to roll their own. It is unlikely that something for 16-bit or even lower is needed (although you would have signed ones) but a 128 bit one would be a nice-to-have for some users. And that would be easy now. Please note that I wrote "and allow the user", so no extra files just a line or two in the documentation pointing to the ability and shoving the macros to something public.

also with respect to #301

Yeah, would get more complicated, would need to run the preprocessor over them, too (or use the Perl module I mentioned elsewhere, but that is not in a standard installation), especially for @minad 's static-fying in put_all_in_one_file.pl (extended version of gen.pl). but I think I can handle it.

Although it would be much easier for me if the macros were already expanded in the individual files ;-)

[minad]

There are five macros for 12 functions of which 6 are essentially one2one, I doubt the ROI is very high in this one ;-)
So: why only 32 and 64 bit? The macros are general enough with your changes—they should go public and allow the user to roll their own. It is unlikely that something for 16-bit or even lower is needed (although you would have signed ones) but a 128 bit one would be a nice-to-have for some users. And that would be easy now. Please note that I wrote "and allow the user", so no extra files just a line or two in the documentation pointing to the ability and shoving the macros to something public.

This is somehow the point. These macros are useful to generate more functions of this class. But I am 100% with you, that as of now the ROI is very low. I expressed my doubts before. However I would still keep them internal for now. If a user is interested, they can take tommath_private.h and generate more functions if needed.

Yeah, would get more complicated, would need to run the preprocessor over them, too (or use the Perl module I mentioned elsewhere, but that is not in a standard installation), especially for @minad 's static-fying in put_all_in_one_file.pl (extended version of gen.pl). but I think I can handle it.
Although it would be much easier for me if the macros were already expanded in the individual files ;-)

Cool, let's discuss this in #301. I am really looking forward to this amalgamation feature + static functions, since this allows to embed tommath.h inside a c file with full global optimization :)

[nijtmans]

Yes, I like it. Something like this I had in mind. Thank you very much!

[czurnieden]

I'm not 100% sure if the last commit generate tommath_class from pre-processed code is a good idea […]

Just sat down to adapt #301 to this PR. Not that much of a problem but: your current version insists on GCC doing the preprocessor work. That is a bit too restrictive, even for me.

Some alternatives:

• ask the user which compiler is installed (just as an option), use GCC (default), or fail (none found)
• use a Perl module like e.g.: Text::CPP. But it is not in CORE and an XS module (needs a compiler to get installed if a binary is not available)
• Only a subset is needed, write that Perl-module yourself. But that's not really an alternative, isn't it?
• drop the autogeneration and use a static list for the functions. Would work for the classes but not for generating mpi.c and I fear the holy wrath of @minad if we do that ;-)
• expand the macros in the individual files at coding-time and keep the macros, because they are useful beyond their current purpose. Don't expand fully, keep the constants like e.g.: MP_DIGIT_BIT for portability.

Although I prefer the last alternative—obviously ;-)—the first one is probably the quickest to implement.

[minad]

@czurnieden what about using env $CC and fallback to gcc? I would like to avoid heavy perl dependencies if possible.

[czurnieden]

what about using env $CC and fallback to gcc

I don't know if env works for other OSs, too. And you would still have to parse the result and keep a list of the options for at least a handful of compilers. It is in most of the cases just a different name/path for GCC or a compatible compiler like clang and I think the output of ICC (Intel's compiler) can also be used but you have to keep a list. Then we have Windows which has its very own theme.
Asking the user for the correct command is easier.

Static files, on the other side, would be completely without any such headaches! ;-)

[minad]

I don't know if env works for other OSs, too. And you would still have to parse the result and keep a list of the options for at least a handful of compilers. It is in most of the cases just a different name/path for GCC or a compatible compiler like clang and I think the output of ICC (Intel's compiler) can also be used but you have to keep a list. Then we have Windows which has its very own theme.
Asking the user for the correct command is easier.

Please don't overcomplicate things. makefile.unix also relies on CC and the file regeneration can always be done in a unix like environment. #301 seems already more complicated in contrast to what we have now, but I have to take a closer look soon.

[minad]

@sjaeckel I took the liberty and added a few commits improving helper.pl - in particular sorting the dependencies in tommath_class.h. This makes it easier to see changes during regeneration. The gcc preprocessing looks good 👍 I think you can merge this. The only downside I am seeing is the slowdown during --update-makefiles.

[minad]

@sjaeckel If you don't like the commits, feel free to revert - I can also move this stuff to another PR if desired.

[karel-m]

Can we also get rid of the trickery with u##type in MP_SET_SIGNED macro?

[minad]

@karel-m done. Now it should be possible to update your no-stdint branch.

[czurnieden]

Please don't overcomplicate things. makefile.unix also relies on CC

(Well, using static files would be the least complicated ;-) )

LTM builds on more OSs than just the Unix-likes. It is more portable to offer an additional option to pass the correct command instead of relying on environment variables only. It's Perl, you won't need to add more than three or four lines to do it.

[sjaeckel]

@fperrad can you please put your linter commits on top, if there are none please only approve :)

[sjaeckel]

btw @karel-m if you add your modification-script to helper.pl we can add more build-jobs to appveyor that first modify the sources and then build them (at least with the msvc available there)

[sjaeckel]

thx btw @minad for the improvements :-)