codeplea / genann

simple neural network library in ANSI C
https://codeplea.com/genann
zlib License
1.99k stars 237 forks source link

link issue using msvc #28

Closed jeog closed 5 years ago

jeog commented 5 years ago

Great work on this, just wanted to make note of a minor issue when using MSVC(compiler v19, linker v14) to compile and link w/ the test file for windows. (gcc on linux worked fine.)

It seems the 'inline' definitions are causing unresolved external symbol errors (e.g genann_act_threshold). I messed around w/ different optimization switches (/GL, /LTCG, /Ob{0|1|2} etc.) but it doesn't seem to help. Making the declarations explicitly 'extern' fixes it, but not sure if that will cause issues elsewhere, or make the 'inline' superfluous .

Or maybe I'm missing something obvious?

codeplea commented 5 years ago

Thanks for the feedback. I'll look into it when I have time.

jflopezfernandez commented 5 years ago

This actually isn't an issue with your compiler; inline functions must be defined in a header file, unless the build has only one compilation unit. This is because there is no function call with an inline function; the compiler literally writes the function in-place (thus the term), which it can't do if it can't see the function definition.

The problem has been solved in two ways. First, I refactored the project and created a build system that builds a shared library. This actually doesn't help you, since I know you're on Windows, though, so the second solution is that I removed the inline specifiers from the functions. You technically did that too when you when you declared the function extern. I can't believe you were able to do that, by the way, that should literally be a compiler error. There is no such thing as an extern inline function, and that's awesome that you did that. You might want to message the MSVC compiler team, I wonder how no one's caught that yet.

The thing is that every function declaration is extern by default. That's why no one uses the specifier anymore. But if you were to say, be trying to fix a compiler error because it couldn't find a function definition, then that would be the place to use it. If you didn't remove the inline specifier, I guess.

More to the point, you have a few options. Move the inline function definitions to the header file, remove the inline specifier, or build a DLL that you link your main program to. There is an initial runtime hit because the executable has to load the library and calculate location offsets, but once it's actually loaded you shouldn't notice any performance penalties.

codeplea commented 5 years ago

Looks like the inline came from PR https://github.com/codeplea/genann/pull/8 Just linking in case @amboar wants to comment. I assume that inline gave some measurable speedup, but I guess it's not worth it if it breaks the build.

codeplea commented 5 years ago

The inlines have been removed.

amboar commented 5 years ago

Removing the inlines seems like the right way to go. Unfortunately I've stopped using genann, so any other of my optimisations are causing grief please feel free to rip them out.