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

Differentiation between multi-threaded and single-threaded preprocessor directives in C #1411

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

erlingrj
Copy link
Collaborator

This PR adds two compiler definitions to the C target: LF_MULTI_THREADED and LF_SINGLE_THREADED which reflect the target property "threading". I have previously used NUMBER_OF_WORKERS to try to get the same info, however, NUMBER_OF_WORKERS is defined even if threading: false, if tracing: true.

Thus I propose to make the fact that we are single-threaded or multi-threaded globally available. It would suffice with only a single flag, but I think it is clearer with a "positive definition" of something than a negative.

@erlingrj erlingrj requested a review from edwardalee October 12, 2022 23:59
Copy link
Collaborator

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay. I'm not sure this change makes sense. I suspect that tracing won't actually work in an unthreaded runtime, otherwise I don't know why it would be setting NUMBER_OF_WORKERS.

@erlingrj
Copy link
Collaborator Author

erlingrj commented Oct 23, 2022

tracing: true sets NUMER_OF_WORKERS=0. But it does not include the threaded runtime. I.e. reactor.c is used, not reactor_threaded.c. I suspect the flag is set because some functionality needed for tracing is hidden behind #ifdef NUMBER_OF_WORKERS. But it does not use the threaded runtime unless threading is specified.

For LET scheduling we need to know, in reactor_common.c whether we are in a single-threaded or multi-threaded runtime. This cannot be checked by #ifdef NUMBER_OF_WORKERS. It could also be checked with

#ifdef NUMBER_OF_WORKERS
#if NUMBER_OF_WORKERS == 0
// single-threaded with tracing enabled
#else
// multi-threaded
#endif
#else
// Single-threaded without tracing
#endif

I.e. I don't really need a new flag, but it seems a bit clunky

@erlingrj erlingrj added the c Related to C target label Oct 23, 2022
@edwardalee
Copy link
Collaborator

Ah, I see. I think the reason is simply that tracing needs a to report a thread number. There may be a better solution, but your solution seems fine, let's merge it.

Copy link
Collaborator

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this.

@erlingrj
Copy link
Collaborator Author

Who can merge this for me? @lhstrh ?

@lhstrh
Copy link
Member

lhstrh commented Nov 1, 2022

@erlingrj: Just resolved a conflict and waiting for CI. We can merge soon as we're green!

@lhstrh lhstrh changed the title C: Add compiler flag to distinguish between multi-threaded and single-threaded runtime Differentiation between multi-threaded and single-threaded preprocessor directives in C Nov 1, 2022
@lhstrh lhstrh merged commit eb29b5a into master Nov 2, 2022
@lhstrh lhstrh deleted the add-threading-flag-cmake branch November 2, 2022 21:58
@lhstrh lhstrh added the refactoring Code quality enhancement label Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c Related to C target refactoring Code quality enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants