Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to make fat externals supporting both single and double precision? #900

Open
claudeha opened this issue Mar 2, 2020 · 14 comments
Open

how to make fat externals supporting both single and double precision? #900

claudeha opened this issue Mar 2, 2020 · 14 comments
Labels

Comments

@claudeha
Copy link
Contributor

@claudeha claudeha commented Mar 2, 2020

I want to make a single binary that loads and functions correctly in Pd compiled with both -DPD_FLOATSIZE=32 and -DPD_FLOATSIZE=64.

I thought about using dlsym() (or Windows equivalent) to lookup class_new vs class_new64 to know the float size of the Pd my external is being loaded into, but it seems Pd defines both (solely in order to print out more friendly error messages than the usual "symbol not found" from the dynamic linker).

I could call both regardless, but that would lead to misleading and confusing error messages to the user.

So I wonder how to get at runtime the PD_FLOATSIZE that Pd has been compiled with, without emitting error messages. Maybe there should be an int sys_get_floatsize(void)?

@Spacechild1

This comment has been minimized.

Copy link
Contributor

@Spacechild1 Spacechild1 commented Mar 2, 2020

Hi, note that you always have to compile your library twice! You can't use the same binary for both single and double precision, since the exported structs and function prototypes in "m_pd.h" also depend on the float size.

I have sketched out a process once, but I can't find it anymore... The idea was to compile your library both as single and double precision and mangle the setup function depending on PD_FLOATSIZE. Then you have a small stub file which contains the actual setup function, which calls both "private" setup functions. One of the two won't load and you get some warnings, but those will only be visible at debug level. Getting the float size at runtime could indeed be more elegant, because then you can call only the right setup function.

On the other hand, maybe we could introduce optional name mangling for the setup function, e.g. mylib_setup_single VS mylib_setup_double and Pd could simply ignore the "wrong" setup function when loading the external. This could be done with a macro, so it would be transparent to the library author:

void PD_SETUP(mylib) (void) { ... }

Just thinking out loud... The notion of coexisting single and double precision Pd and externals raises a couple of interesting questions :-)

@claudeha

This comment has been minimized.

Copy link
Contributor Author

@claudeha claudeha commented Mar 2, 2020

I think I found the sketch: https://lists.puredata.info/pipermail/pd-dev/2019-12/022214.html

If name mangling, I think the mangled name for PD_FLOATSIZE=32 should be the same name as historically used (ie, mylib_setup) to avoid having to recompile everything.

@Spacechild1

This comment has been minimized.

Copy link
Contributor

@Spacechild1 Spacechild1 commented Mar 2, 2020

Fair enough. Name mangling could actually make the whole current solution with class_new64 obsolete (although I appreciate the cleverness :-)... I'm curious to hear @umlaeute's opinion.

@claudeha

This comment has been minimized.

Copy link
Contributor Author

@claudeha claudeha commented Mar 2, 2020

I think name mangling without class_new64 would be my preference too, having just discovered that fat externals won't load in old pd (dl error missing symbol class_new64)...

@claudeha

This comment has been minimized.

Copy link
Contributor Author

@claudeha claudeha commented Mar 2, 2020

agraef/pd-lua@3ef4671 is what I had to do with the ecosystem as of today, to get a fat binary that works in both old Pd (tested with Debian Buster, PD_FLOATSIZE=32, no symbol class_new64) and new Pd (tested with 0.50.2+git with PD_FLOATSIZE=64). I'm really hoping this can be simplified for external authors, because these hacks aren't great fun, and will inhibit the adoption of fat external builds.

@umlaeute

This comment has been minimized.

Copy link
Contributor

@umlaeute umlaeute commented Mar 2, 2020

i dislike the idea of float-size specific entry-points into the library (mylib_setup_single).

The reason is simple, that this way there is no way to make sure an external doesn't register a class with the wrong float-size.
It's pretty simple to circumvent the setup mechanism alltogether and just call whatever class_new you like while the external is being bootstrapped.

The fact that Pd-0.50 doesn't export the class_new64 function is of course a problem.
For whatever reasons my original patchset was only included halfways into Pd-0.50, so defining PD_FLOATSIZE=64 will turn your class_new into class_new64, but that function doesn't get defined in a single-precision runtime :-(

One possibility to fix the issue would be to use weak linking.
something like this (in m_pd.h):

EXTERN t_class *class_new64(t_symbol *name, t_newmethod newmethod,
    t_method freemethod, size_t size, int flags, t_atomtype arg1, ...) __attribute__((__weak__));
// ...
#if PD_FLOATSIZE == 64
# define class_new (!class_new64)?0:class_new64
#endif

i'm not aware whether windows actually has something like weak linking.

we probably also need a sys_getfloatsize() function.

@Spacechild1

This comment has been minimized.

Copy link
Contributor

@Spacechild1 Spacechild1 commented Mar 2, 2020

The reason is simple, that this way there is no way to make sure an external doesn't register a class with the wrong float-size.
It's pretty simple to circumvent the setup mechanism alltogether and just call whatever class_new you like while the external is being bootstrapped.

With name mangling there would be only one class_new, but it requires that the setup function is wrapped in a macro like PD_SETUP which expands to either mylib_setup or mylib_setup_double depending on PD_FLOATSIZE. As long as you don't forget the macro, then this is pretty safe.

You can then simply compile the library twice - with PD_FLOATSIZE set to 32 resp. 64 - and link the two object files together. The shared lib will contain both versions and export two setup functions. (Of course, you have to make sure that all other functions are static to avoid linker errors). Or you can build it as two separate shared objects.

The only downside I see is that existing externals need a small modification (the PD_SETUP macro for setup functions), which some may find annoying. Apart from that, it seems pretty clean to me...

Anyway, I think there are many open questions regarding how double precision externals should be build and distributed. What's the best way to discuss such important design decisions?

@Lucarda

This comment has been minimized.

Copy link
Contributor

@Lucarda Lucarda commented Mar 2, 2020

Anyway, I think there are many open questions regarding how double precision externals should be build and distributed. What's the best way to discuss such important design decisions?

Given that all C experts are kind of full of doubts and uncertainty of how this could be done, I propose that there should not be FAT binaries.

Pd should simply have an different extension for "double externals" alla foo.l_amd64-DP, foo.m_amd64-DP and so on.

I think this the less confusing option for everybody: devs or users.

@Lucarda

This comment has been minimized.

Copy link
Contributor

@Lucarda Lucarda commented Mar 2, 2020

My proposal of the different file extension is somehow biased because I'm willing to provide windows-double binaries to mostly all the deken w64 catalogue.

@umlaeute

This comment has been minimized.

Copy link
Contributor

@umlaeute umlaeute commented Mar 5, 2020

@Lucarda since this ticket is about "how to create fat externals", i think your filename suggestions should go into a separate ticket.

@umlaeute

This comment has been minimized.

Copy link
Contributor

@umlaeute umlaeute commented Mar 5, 2020

@Spacechild1 wrote:

With name mangling there would be only one class_new, but it requires that the setup function is wrapped in a macro like PD_SETUP which expands to either mylib_setup or mylib_setup_double depending on PD_FLOATSIZE. As long as you don't forget the macro, then this is pretty safe.

as i've tried to explain in https://lists.puredata.info/pipermail/pd-dev/2020-03/022327.html, mangling class_new and mangling setup are two different things that attempt to solve different problems.

the usual way to register new classes is in the setup function, but there is absolutely nothing that enforces this.
therefore, we cannot just get rid of class_new64. (i haven't found another way to ensure that Pd knows the precision of a given objectclass; one possibility would have been to use the flags argument and make CLASS_DEFAULT indicate the precision; but since virtually all externals use 0 instead of CLASS_DEFAULT it's of no practical use; also (an older) Pd would happily ignore any additional flags, and then crash with double-precision externals)

otoh, i understand that mangling the setup-function makes it (much) easier for external developers to provide phat binaries.
we probably should use both class_new64() to make sur ethat Pd knows the precision of an objectclass, and <library>_setup_float64() (resp setup_<hexmungedlibrary>_float64()) to make it easy for externals to call the correct class_new*() variant.

i'd also like to get some feedback on the weak-linking proposed in https://git.iem.at/pd/pure-data/-/blob/tests/double-API/src/m_pd.h#L933-981 to allow older versions of Pd to load phat binaries.

@Spacechild1

This comment has been minimized.

Copy link
Contributor

@Spacechild1 Spacechild1 commented Mar 6, 2020

the usual way to register new classes is in the setup function, but there is absolutely nothing that enforces this.

I don't see how that can happen with mandatory name mangling for double precision externals (apart from the external author/builder forgetting to mangle the setup function). Can you give a practical example?

I'm not against the class_new64 trick, I just think that with namemangling it might not be necessary and we wouldn't need the weak linking hack for older Pd binaries (I have to look if this is even possible on Windows).

@umlaeute

This comment has been minimized.

Copy link
Contributor

@umlaeute umlaeute commented Mar 6, 2020

I don't see how that can happen with mandatory name mangling for double precision externals (apart from the external author/builder forgetting to mangle the setup function). Can you give a practical example?

e.g. a "loader" external that creates objectclasses on user-request (and not while Pd is looking for an object with a given name).

weak linking hack for older Pd binaries (I have to look if this is even possible on Windows).

check out my branch. it seems to work fine on Windows (at least with wine...)

@Spacechild1

This comment has been minimized.

Copy link
Contributor

@Spacechild1 Spacechild1 commented Mar 6, 2020

e.g. a "loader" external that creates objectclasses on user-request

How does it bypass the setup function? Is there a concrete example in the wild?

check out my branch. it seems to work fine on Windows (at least with wine...)

I will!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.