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

Replacing hpx_main's #define main with overriding __libc_start_main #3338

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@NK-Nikunj
Contributor

NK-Nikunj commented May 28, 2018

This is my first GSoC'18 commit. Currently, this commit provides code for refining hpx_main.hpp for gcc and clang based compilers. I am working on msvc based compilers and will provide code for the same soon.

Proposed Changes

  • Refining hpx_main.hpp by using wrapper for __libc_start_main

Any background context you want to provide?

I have created a wrapper for __libc_start_main and created a hook for the function. I changed the signature of __libc_Start_main by providing my own entry point (__initializing_main) instead of main. From here I call the hpx::init to start the HPX runtime system. From the initialized system, I call the main function.

For a user, including this header will enable him to use HPX functionality directly from int main.

Current challenges and refinement required

This implementation has a few drawbacks which I'm currently working on. These are as follows:

  • It will NOT work for any global object using HPX functionality.
  • It has used global scope variables, which in my opinion is not a good practice

Any suggestions and reviews are welcomed.

// We support redefining the plain C-main provided by the user to be executed
// as the first HPX-thread (equivalent to hpx_main()). This is implemented by
// a macro redefining main, so we disable it by default.
#define main hpx_startup::user_main
#endif

This comment has been minimized.

@hkaiser

hkaiser May 28, 2018

Member

This disables the #define main on all platforms that do not support your new initialization code. I think the static linking is orthogonal to your new functionality.

This comment has been minimized.

@NK-Nikunj

NK-Nikunj May 28, 2018

Contributor

@hkaiser yes, static linking is orthogonal to my implementation. I should probably change it to gnuc, clang and not HPX_HAVE_STATIC_LINKING and keep the rest same. This should solve the issue from what I think.

endforeach()
else()
message(FATAL_ERROR "This test is meant only for gcc and clang compilers.")

This comment has been minimized.

@hkaiser

hkaiser May 28, 2018

Member

If you unconditionally raise an error on MSVC it will break the whole build system. I think there is no need for this else() case altogether.

This comment has been minimized.

@NK-Nikunj

NK-Nikunj May 28, 2018

Contributor

It was added as a precaution for msvc based build which will currently not work. But yes it is redundant and can be removed. Since it will cause the whole system to break, I'll remove it altogether.

// Simple test verifying runtime initialization sequence
#define _GNU_SOURCE

This comment has been minimized.

@hkaiser

hkaiser May 28, 2018

Member

Why is this necessary?

This comment has been minimized.

@NK-Nikunj

NK-Nikunj May 28, 2018

Contributor

@hkaiser I added it to write portable code. It provides access to various nonstandard GNU/Linux extension functions. I have not made any use of it, but I added keeping in mind that I might need it in the final implementation.

This comment has been minimized.

@hkaiser

hkaiser May 28, 2018

Member

I don't think it to be a good idea to request from the user to define this in order for your system to work. Wouldn't there be a way to just #include <hpx/hpx_main.hpp> and be done with it?

This comment has been minimized.

@NK-Nikunj

NK-Nikunj May 28, 2018

Contributor

I was firstly creating another test which included what's written in #include <hpx/hpx_main.hpp> so I included these instead of hpx_main.hpp. It is not necessary here, neither is dlfcn.h. It will work simply by including hpx_main.hpp.

This comment has been minimized.

@NK-Nikunj

NK-Nikunj May 28, 2018

Contributor

I will make the changes accordingly.

#define _GNU_SOURCE
#include <dlfcn.h>

This comment has been minimized.

@hkaiser

hkaiser May 28, 2018

Member

Why is this #include needed?

This comment has been minimized.

@NK-Nikunj

NK-Nikunj May 28, 2018

Contributor

@hkaiser This is necessary for using the function dlsym that I'm using to get the pointer to __libc_start_main.

This comment has been minimized.

@hkaiser

hkaiser May 28, 2018

Member

Same comment as above. The user shouldn't see all of this.

@hkaiser

This comment has been minimized.

Member

hkaiser commented May 28, 2018

Another question: have the function implementations (i.e. __hpx_entry etc.) to be inlined?

@NK-Nikunj

This comment has been minimized.

Contributor

NK-Nikunj commented May 28, 2018

Not necessarily. But I can make them inline if you want me to.

@hkaiser

This comment has been minimized.

Member

hkaiser commented Jun 2, 2018

Using names starting with '__' are reserved for the implementation, could you use names without that please (feel free to move the functions into a namespace, though)?

@NK-Nikunj

This comment has been minimized.

Contributor

NK-Nikunj commented Jun 27, 2018

Closing due to better implementation in the pr #3357

@NK-Nikunj NK-Nikunj closed this Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment