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

[Libomptarget] Rework Record & Replay to be a plugin member #88928

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Apr 16, 2024

Summary:
Previously, the R&R support was global state initialized by a global
constructor. This is bad because it prevents us from adequately
constraining the lifetime of the library. Additionally, we want to
minimize the amount of global state floating around.

This patch moves the R&R support into a plugin member like everything
else. This means there will be multiple copies of the R&R implementation
floating around, but this was already the case given the fact that we
currently handle everything with dynamic libraries.

Copy link
Contributor

@koparasy koparasy left a comment

Choose a reason for hiding this comment

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

Overall it LGTM. Some minor comments.

@jhuber6 jhuber6 force-pushed the RecordAndReplay branch 2 times, most recently from a30737a to 8b721d7 Compare April 16, 2024 16:54
Copy link
Contributor

@ggeorgakoudis ggeorgakoudis left a comment

Choose a reason for hiding this comment

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

LGTM, one comment

Summary:
Previously, the R&R support was global state initialized by a global
constructor. This is bad because it prevents us from adequately
constraining the lifetime of the library. Additionally, we want to
minimize the amount of global state floating around.

This patch moves the R&R support into a plugin member like everything
else. This means there will be multiple copies of the R&R implementation
floating around, but this was already the case given the fact that we
currently handle everything with dynamic libraries.
@jhuber6 jhuber6 merged commit 9a0a28f into llvm:main Apr 16, 2024
4 checks passed
@jplehr
Copy link
Contributor

jplehr commented Apr 17, 2024

This apparently broke all our buildbots, example: https://lab.llvm.org/buildbot/#/builders/193/builds/50201

jplehr added a commit that referenced this pull request Apr 17, 2024
jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Apr 17, 2024
Summary:
Previously, the R&R support was global state initialized by a global
constructor. This is bad because it prevents us from adequately
constraining the lifetime of the library. Additionally, we want to
minimize the amount of global state floating around.

This patch moves the R&R support into a plugin member like everything
else. This means there will be multiple copies of the R&R implementation
floating around, but this was already the case given the fact that we
currently handle everything with dynamic libraries.
jhuber6 added a commit to jhuber6/llvm-project that referenced this pull request Apr 17, 2024
Summary:
Previously, the R&R support was global state initialized by a global
constructor. This is bad because it prevents us from adequately
constraining the lifetime of the library. Additionally, we want to
minimize the amount of global state floating around.

This patch moves the R&R support into a plugin member like everything
else. This means there will be multiple copies of the R&R implementation
floating around, but this was already the case given the fact that we
currently handle everything with dynamic libraries.
jhuber6 added a commit that referenced this pull request May 16, 2024
…89097)

Summary:
Previously, the R&R support was global state initialized by a global
constructor. This is bad because it prevents us from adequately
constraining the lifetime of the library. Additionally, we want to
minimize the amount of global state floating around.

This patch moves the R&R support into a plugin member like everything
else. This means there will be multiple copies of the R&R implementation
floating around, but this was already the case given the fact that we
currently handle everything with dynamic libraries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomptarget OpenMP offload runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants