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

[Profile][Windows] Drop extern for __buildid. #80700

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

ZequanWu
Copy link
Contributor

@ZequanWu ZequanWu commented Feb 5, 2024

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 5, 2024

@llvm/pr-subscribers-pgo

Author: Zequan Wu (ZequanWu)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/80700.diff

1 Files Affected:

  • (modified) compiler-rt/lib/profile/InstrProfilingPlatformWindows.c (+1-1)
diff --git a/compiler-rt/lib/profile/InstrProfilingPlatformWindows.c b/compiler-rt/lib/profile/InstrProfilingPlatformWindows.c
index c976776ae59e9..0751b28f81d0a 100644
--- a/compiler-rt/lib/profile/InstrProfilingPlatformWindows.c
+++ b/compiler-rt/lib/profile/InstrProfilingPlatformWindows.c
@@ -77,7 +77,7 @@ ValueProfNode *EndVNode = &VNodesEnd;
 /* lld-link provides __buildid symbol which ponits to the 16 bytes build id when
  * using /build-id flag. https://lld.llvm.org/windows_support.html#lld-flags */
 #define BUILD_ID_LEN 16
-COMPILER_RT_WEAK extern uint8_t __buildid[BUILD_ID_LEN];
+COMPILER_RT_WEAK uint8_t __buildid[BUILD_ID_LEN];
 COMPILER_RT_VISIBILITY int __llvm_write_binary_ids(ProfDataWriter *Writer) {
   if (*__buildid) {
     if (Writer &&

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM, I confirmed that this fixes the mingw build.

@ZequanWu ZequanWu merged commit dd22140 into llvm:main Feb 5, 2024
7 checks passed
@mati865
Copy link
Contributor

mati865 commented Feb 5, 2024

Can we also backport it to 18 release?

also cc @mstorsjo for awareness

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 6, 2024
@mstorsjo
Copy link
Member

mstorsjo commented Feb 6, 2024

Just for reference - see #75618 (comment) for a bit more discussion on the solution chosen here. (In short, the chosen solution is fine.)

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 6, 2024
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
@ZequanWu ZequanWu deleted the drop-extern branch February 15, 2024 18:31
@pointhex pointhex mentioned this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants