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

[PGO][compiler-rt] Add a test to ensure include files do not diverge #97775

Merged

Conversation

boomanaiden154
Copy link
Contributor

@boomanaiden154 boomanaiden154 commented Jul 4, 2024

Memprof has two include files that are duplicated between LLVM and compiler-rt. They need to stay in sync to ensure correct functionality, but the comments can be somewhat easy to miss, which causes fixups like 839ed1b to be needed. This patch adds a test to ensure that the files are the same between LLVM and compiler-rt to catch this ideally before commit, but if not, soon afterwards.

There is additionally InstrProfData.inc for some PGO variants that is added to the same test here as well.

Memprof has two include files that are duplicated between LLVM and
compiler-rt. They need to stay in sync to ensure correct functionality,
but the comments can be somewhat easy to miss, which causes fixups like
839ed1b to be needed. This patch adds a
test to ensure that the files are the same between LLVM and compiler-rt
to catch this ideally before commit, but if not, soon afterwards.
@llvmbot llvmbot added compiler-rt PGO Profile Guided Optimizations labels Jul 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 4, 2024

@llvm/pr-subscribers-pgo

Author: Aiden Grossman (boomanaiden154)

Changes

Memprof has two include files that are duplicated between LLVM and compiler-rt. They need to stay in sync to ensure correct functionality, but the comments can be somewhat easy to miss, which causes fixups like 839ed1b to be needed. This patch adds a test to ensure that the files are the same between LLVM and compiler-rt to catch this ideally before commit, but if not, soon afterwards.


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

1 Files Affected:

  • (added) compiler-rt/test/profile/check-same-common-code.test (+6)
diff --git a/compiler-rt/test/profile/check-same-common-code.test b/compiler-rt/test/profile/check-same-common-code.test
new file mode 100644
index 0000000000000..5cede37fe06c2
--- /dev/null
+++ b/compiler-rt/test/profile/check-same-common-code.test
@@ -0,0 +1,6 @@
+;
+; NOTE: if this test fails, please make sure the relevant copies are identical
+; copies of each other.
+;
+; RUN: diff %crt_src/include/profile/MIBEntryDef.inc %llvm_src/include/llvm/ProfileData/MIBEntryDef.inc
+; RUN: diff %crt_src/include/profile/MemProfData.inc %llvm_src/include/llvm/ProfileData/MemProfData.inc

@boomanaiden154 boomanaiden154 changed the title [MemProf][compiler-rt] Add a test to ensure include files do not diverge [PGO][compiler-rt] Add a test to ensure include files do not diverge Jul 5, 2024
Copy link
Contributor

@snehasish snehasish left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for adding a test to keep things consistent.

compiler-rt/test/profile/check-same-common-code.test Outdated Show resolved Hide resolved
@boomanaiden154 boomanaiden154 merged commit 44248d2 into llvm:main Jul 5, 2024
@boomanaiden154 boomanaiden154 deleted the memprof-compiler-rt-test-diff branch July 5, 2024 17:56
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
…lvm#97775)

Memprof has two include files that are duplicated between LLVM and
compiler-rt. They need to stay in sync to ensure correct functionality,
but the comments can be somewhat easy to miss, which causes fixups like
839ed1b to be needed. This patch adds a
test to ensure that the files are the same between LLVM and compiler-rt
to catch this ideally before commit, but if not, soon afterwards.

There is additionally `InstrProfData.inc` for some PGO variants that is
added to the same test here as well.
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.

3 participants