Allow mixing debug and release builds #5099

msimberg · 2021-01-05T16:11:25Z

Testing if this is feasible. In principle it is, but I'd appreciate feedback on if there are any pitfalls that I'm not aware of, especially on Windows. Ideally we only use HPX_DEBUG determine if parts of the code should be conditionally compiled/disabled/etc. However, there are a few additional macros that we use:

DEBUG: I could find no reference to anything using this, does anyone know what it's doing in HPX?

_DEBUG: Windows specific, added automatically in debug mode, correct? There are a couple of uses of this one. Can we replace them with HPX_DEBUG? (

hpx/src/runtime_distributed.cpp

Lines 546 to 551 in 8cb1829

    
           #if defined(_WIN64) && defined(_DEBUG) &&                                      \ 
        
               !defined(HPX_HAVE_FIBER_BASED_COROUTINES) 
        
                   // needs to be called to avoid problems at system startup 
        
                   // see: http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=100319 
        
                   _isatty(0); 
        
           #endif

)

NDEBUG: Disables assert. We have a few uses of assert which are fine, but as far as I can tell there is no reason for us to rely on this macro.
HPX_DISABLE_ASSERTIONS/BOOST_DISABLE_ASSERTIONS: Used to disable a few assertions, but not HPX_ASSERT. Used together with NDEBUG in some cases. I'd at most leave HPX_DISABLE_ASSERTIONS.

Is there a use case for debug builds with assertions disabled? If yes, should it get a CMake option instead of needing to be manually added to the compile flags?

hkaiser · 2021-01-05T16:44:28Z

DEBUG: I could find no reference to anything using this, does anyone know what it's doing in HPX?

There is no reason to use this anywhere, even more as we map it onto HPX_DEBUG (https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/config/include/hpx/config/debug.hpp#L19-L25)

_DEBUG: Windows specific, added automatically in debug mode, correct?

Yes, this auto-added in Debug builds. It is mapped onto HPX_DEBUG, also here: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/config/include/hpx/config/debug.hpp#L19-L25

There are a couple of uses of this one. Can we replace them with HPX_DEBUG?

Yes.

NDEBUG: Disables assert. We have a few uses of assert which are fine, but as far as I can tell there is no reason for us to rely on this macro.

This is auto-added on windows for non-debug builds, there shouldn't be any reason for us to use it. Also, we should replace all assert() with HPX_ASSERT(). Actually, I thought we have an inspect check for this.

HPX_DISABLE_ASSERTIONS/BOOST_DISABLE_ASSERTIONS: Used to disable a few assertions, but not HPX_ASSERT. Used together with NDEBUG in some cases. I'd at most leave HPX_DISABLE_ASSERTIONS.

Is there a use case for debug builds with assertions disabled? If yes, should it get a CMake option instead of needing to be manually added to the compile flags?

I think this is a leftover from times when this macro caused HPX_ASSERT to fallback to assert. I think this can be removed altogether.

msimberg · 2021-01-06T08:33:02Z

DEBUG: I could find no reference to anything using this, does anyone know what it's doing in HPX?

There is no reason to use this anywhere, even more as we map it onto HPX_DEBUG (https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/config/include/hpx/config/debug.hpp#L19-L25)

_DEBUG: Windows specific, added automatically in debug mode, correct?

Yes, this auto-added in Debug builds. It is mapped onto HPX_DEBUG, also here: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/config/include/hpx/config/debug.hpp#L19-L25

There are a couple of uses of this one. Can we replace them with HPX_DEBUG?

Yes.

NDEBUG: Disables assert. We have a few uses of assert which are fine, but as far as I can tell there is no reason for us to rely on this macro.

This is auto-added on windows for non-debug builds, there shouldn't be any reason for us to use it. Also, we should replace all assert() with HPX_ASSERT(). Actually, I thought we have an inspect check for this.

HPX_DISABLE_ASSERTIONS/BOOST_DISABLE_ASSERTIONS: Used to disable a few assertions, but not HPX_ASSERT. Used together with NDEBUG in some cases. I'd at most leave HPX_DISABLE_ASSERTIONS.

Is there a use case for debug builds with assertions disabled? If yes, should it get a CMake option instead of needing to be manually added to the compile flags?

I think this is a leftover from times when this macro caused HPX_ASSERT to fallback to assert. I think this can be removed altogether.

Perfect, thanks @hkaiser for confirming all these!

_DEBUG: Windows specific, added automatically in debug mode, correct?

Yes, this auto-added in Debug builds. It is mapped onto HPX_DEBUG, also here: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/config/include/hpx/config/debug.hpp#L19-L25

I was hoping I could remove that mapping (it's already gone on this branch), since with the mapping we can still end up with inconsistent flags (user code code compiled with HPX_DEBUG defined and HPX without it).

msimberg · 2021-01-06T08:39:30Z

NDEBUG: Disables assert. We have a few uses of assert which are fine, but as far as I can tell there is no reason for us to rely on this macro.

This is auto-added on windows for non-debug builds, there shouldn't be any reason for us to use it. Also, we should replace all assert() with HPX_ASSERT(). Actually, I thought we have an inspect check for this.

This is not as bad as it sounds. I rechecked and we have it in the moodycamel queue header. It's already excluded from inspect. It might be worth leaving the asserts for easier merging of new versions, but then again it already has formatting and a few other changes to make it different so we could also change them. Don't really mind.

The other place where we use assert is for assertions in device code, which I also think is fine.

cmake/HPX_CompilerFlagsTargets.cmake

cmake/templates/HPXMacros.cmake.in

hkaiser

Should we add CI configurations making sure using a Debug version from a release build and v.v. is actually functional? I'd like to see that for all major platforms (at least Linux and Windows).

msimberg · 2021-01-07T11:56:39Z

Should we add CI configurations making sure using a Debug version from a release build and v.v. is actually functional? I'd like to see that for all major platforms (at least Linux and Windows).

That's my last concern. The build unit tests now build the hello world component example in four different build types. It's not exhaustive but it's a start. A more comprehensive test would be to make use of the feature to build tests against an installed HPX. Let's at the very least wait until we can get the windows builders running again.

Edit: I'm not in a rush to get this in by the way. If we can find a satisfactory way to test this before 1.6.0 we can go ahead with it but it can also wait.

msimberg · 2021-01-08T07:52:54Z

retest lsu

hkaiser · 2021-01-27T23:22:53Z

This issue is not directly connected, but it's related: STEllAR-GROUP/phylanx#1311

cmake/templates/HPXMacros.cmake.in

msimberg · 2021-02-11T08:31:26Z

@hkaiser any idea what I might've missed here: https://github.com/STEllAR-GROUP/hpx/pull/5099/checks?check_run_id=1870902759#step:5:1476?

hkaiser · 2021-02-11T13:16:07Z

@hkaiser any idea what I might've missed here: https://github.com/STEllAR-GROUP/hpx/pull/5099/checks?check_run_id=1870902759#step:5:1476?

Let me try to reproduce this locally.

hkaiser · 2021-02-11T16:59:48Z

@hkaiser any idea what I might've missed here: https://github.com/STEllAR-GROUP/hpx/pull/5099/checks?check_run_id=1870902759#step:5:1476?

Let me try to reproduce this locally.

Looks like that the same commands just work fine for me locally. So it has to be some resource restriction imposed by github. We could try to use -verbosity:detailed (instead of the -verbosity:minimal) and hope to see more information.

msimberg · 2021-02-11T17:07:47Z

@hkaiser any idea what I might've missed here: https://github.com/STEllAR-GROUP/hpx/pull/5099/checks?check_run_id=1870902759#step:5:1476?

Let me try to reproduce this locally.

Looks like that the same commands just work fine for me locally. So it has to be some resource restriction imposed by github. We could try to use -verbosity:detailed (instead of the -verbosity:minimal) and hope to see more information.

Thanks for checking. I'll try that out.

hkaiser · 2021-02-11T20:32:39Z

@msimberg I pushed some changes to this branch that work around an MSVC issue that suddenly manifested itself on this branch. Let's see if that fixes the problems for the Windows builders,

hkaiser · 2021-02-11T21:59:18Z

FWIW, I think the remaining issue is caused by the additional targets you added (cmake_debug_build_dir_targets_test and friends).

- this adds a workaround preventing MSVC from prematurely instantiating templates

.github/workflows/windows_release.yml

hkaiser · 2021-02-13T18:09:01Z

@msimberg this looks like it's good to go.

msimberg · 2021-02-15T08:43:33Z

retest lsu

msimberg · 2021-02-15T08:43:48Z

@msimberg this looks like it's good to go.

Thank you @hkaiser for helping with this!

rbyshko · 2021-03-11T15:41:34Z

I have installed hpx 1.6.0 using vcpkg and tried to build a "Hello world" program as in Quick Start from the documentation. Unfortunately, I still get the error this issue claims to have fixed.

With vcpkg I have also installed the current master (4d74809), but the error is still there.

terminate called after throwing an instance of 'hpx::detail::exception_with_info<hpx::exception>'
  what():  failed to insert base_set_event_action into typename to id registry.: HPX(invalid_status)

Let me know if I can provide more information to get this fixed.

msimberg · 2021-03-11T15:57:56Z

Since you mention vcpkg I'm guessing you're on Windows, is that correct? If that's the case this PR does not allow mixing build types on Windows. I don't know if we can do more to detect these situations, any help with that would definitely be appreciated.

rbyshko · 2021-03-11T17:34:27Z

No, I am using vcpkg on Linux, Ubuntu 20.04 to be exact. vcpkg installs release and debug versions of the hpx. However they go into separate directories. CMake seems to handle it correctly:

msimberg · 2021-03-11T19:01:55Z

No, I am using vcpkg on Linux, Ubuntu 20.04 to be exact. vcpkg installs release and debug versions of the hpx. However they go into separate directories. CMake seems to handle it correctly:

Ok, thanks for the details! That does sound like something I've missed in that case, I'll try to have a look at this soon. As you can see this is rather new, and our testing (clearly) isn't thorough enough for it.

hkaiser · 2021-03-11T19:18:16Z

I have installed hpx 1.6.0 using vcpkg and tried to build a "Hello world" program as in Quick Start from the documentation. Unfortunately, I still get the error this issue claims to have fixed.

With vcpkg I have also installed the current master (4d74809), but the error is still there.
terminate called after throwing an instance of 'hpx::detail::exception_with_info<hpx::exception>'
  what():  failed to insert base_set_event_action into typename to id registry.: HPX(invalid_status)
Let me know if I can provide more information to get this fixed.

This error message indicates that both versions, release and debug, have been loaded. Apparently we still miss something as it shouldn't actually happen. The plugin API we use relies on different function names for release and debug, so there is at least a potential way of preventing to load a mismatched version.

msimberg · 2021-03-11T20:43:47Z

@rbyshko just for completeness, could you provide the code and CMakeLists.txt that you used? And are you seeing this problem with user code in release mode and HPX in debug mode, vice versa, or both?

rbyshko · 2021-03-12T09:50:23Z

I am experiencing this error in both release and debug builds.

CMakeLists.txt

cmake_minimum_required(VERSION 3.16)

set(CMAKE_TOOLCHAIN_FILE ${CMAKE_CURRENT_LIST_DIR}/vcpkg/scripts/buildsystems/vcpkg.cmake)

project(HPX)

find_package(HPX CONFIG REQUIRED)

add_executable(hpx_hello hpx_hello.cpp)
target_link_libraries(hpx_hello PRIVATE HPX::hpx HPX::wrap_main HPX::iostreams_component)

hpx_hello.cpp

#include <hpx/hpx_main.hpp>
#include <hpx/iostream.hpp>

int main()
{
    // Say hello to the world!
    hpx::cout << "Hello World!\n" << hpx::flush;
    return 0;
}

msimberg added category: CMake type: cleanup labels Jan 5, 2021

msimberg force-pushed the mix-debug-release branch 4 times, most recently from 3d5207c to 8c7b0a2 Compare January 6, 2021 11:52

msimberg marked this pull request as ready for review January 7, 2021 09:43

msimberg requested review from aurianer, biddisco and hkaiser as code owners January 7, 2021 09:43

hkaiser reviewed Jan 7, 2021

View reviewed changes

cmake/HPX_CompilerFlagsTargets.cmake Outdated Show resolved Hide resolved

hkaiser reviewed Jan 7, 2021

View reviewed changes

cmake/templates/HPXMacros.cmake.in Show resolved Hide resolved

hkaiser reviewed Jan 7, 2021

View reviewed changes

msimberg force-pushed the mix-debug-release branch 2 times, most recently from 012911e to 2cb375f Compare January 7, 2021 16:09

msimberg added this to the 1.7.0 milestone Jan 13, 2021

msimberg commented Feb 8, 2021

View reviewed changes

cmake/templates/HPXMacros.cmake.in Show resolved Hide resolved

msimberg force-pushed the mix-debug-release branch 2 times, most recently from 452de67 to 5f3ef5f Compare February 10, 2021 11:38

hkaiser force-pushed the mix-debug-release branch 2 times, most recently from d757c14 to f993d40 Compare February 12, 2021 01:48

msimberg and others added 3 commits February 12, 2021 09:42

Allow mixing debug and release builds

f191ab0

Increase verbosity on Windows builders

6a0f823

Fixing MSVC compilation issues

841ed82

- this adds a workaround preventing MSVC from prematurely instantiating templates

msimberg force-pushed the mix-debug-release branch from f993d40 to 690852a Compare February 12, 2021 08:44

Fixing build targets

5bd90a8

hkaiser force-pushed the mix-debug-release branch from 690852a to 5bd90a8 Compare February 12, 2021 13:14

msimberg commented Feb 12, 2021

View reviewed changes

.github/workflows/windows_release.yml Show resolved Hide resolved

msimberg merged commit baffc51 into STEllAR-GROUP:master Feb 15, 2021

msimberg deleted the mix-debug-release branch February 15, 2021 14:37

msimberg mentioned this pull request Mar 16, 2021

Make sure HPX_DEBUG is set based on HPX's build type, not consuming project's build type #5243

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow mixing debug and release builds #5099

Allow mixing debug and release builds #5099

msimberg commented Jan 5, 2021

hkaiser commented Jan 5, 2021

msimberg commented Jan 6, 2021

msimberg commented Jan 6, 2021

hkaiser left a comment

msimberg commented Jan 7, 2021 •

edited

Loading

msimberg commented Jan 8, 2021

hkaiser commented Jan 27, 2021

msimberg commented Feb 11, 2021

hkaiser commented Feb 11, 2021

hkaiser commented Feb 11, 2021

msimberg commented Feb 11, 2021

hkaiser commented Feb 11, 2021

hkaiser commented Feb 11, 2021

hkaiser commented Feb 13, 2021

msimberg commented Feb 15, 2021

msimberg commented Feb 15, 2021 •

edited

Loading

rbyshko commented Mar 11, 2021

msimberg commented Mar 11, 2021

rbyshko commented Mar 11, 2021

msimberg commented Mar 11, 2021

hkaiser commented Mar 11, 2021

msimberg commented Mar 11, 2021

rbyshko commented Mar 12, 2021

	#if defined(_WIN64) && defined(_DEBUG) && \
	!defined(HPX_HAVE_FIBER_BASED_COROUTINES)
	// needs to be called to avoid problems at system startup
	// see: http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=100319
	_isatty(0);
	#endif

Allow mixing debug and release builds #5099

Allow mixing debug and release builds #5099

Conversation

msimberg commented Jan 5, 2021

hkaiser commented Jan 5, 2021

msimberg commented Jan 6, 2021

msimberg commented Jan 6, 2021

hkaiser left a comment

Choose a reason for hiding this comment

msimberg commented Jan 7, 2021 • edited Loading

msimberg commented Jan 8, 2021

hkaiser commented Jan 27, 2021

msimberg commented Feb 11, 2021

hkaiser commented Feb 11, 2021

hkaiser commented Feb 11, 2021

msimberg commented Feb 11, 2021

hkaiser commented Feb 11, 2021

hkaiser commented Feb 11, 2021

hkaiser commented Feb 13, 2021

msimberg commented Feb 15, 2021

msimberg commented Feb 15, 2021 • edited Loading

rbyshko commented Mar 11, 2021

msimberg commented Mar 11, 2021

rbyshko commented Mar 11, 2021

msimberg commented Mar 11, 2021

hkaiser commented Mar 11, 2021

msimberg commented Mar 11, 2021

rbyshko commented Mar 12, 2021

msimberg commented Jan 7, 2021 •

edited

Loading

msimberg commented Feb 15, 2021 •

edited

Loading