[llvm-exegesis][unittests] Also disable SubprocessMemoryTest on SPARC #102755
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… hosts
Three `llvm-exegesis` tests
```
LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/DefinitionFillsCompletely
LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/MultipleDefinitions
LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/OneDefinition
```
`FAIL` on Linux/sparc64 like
```
llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp:68: Failure
Expected equality of these values:
SharedMemoryMapping[I]
Which is: '\0'
ExpectedValue[I]
Which is: '\xAA' (170)
```
It seems like this test only works on little-endian hosts: three
sub-tests are already disabled on powerpc and s390x (both big-endian), and
the fourth is additionaly guarded against big-endian hosts (making the
other guards unnecessary).
Rather than add big-endian hosts (like sparc) one by one to every sub-test,
it seems better to disable the whole test on big-endian.
Tested on `sparc64-unknown-linux-gnu` and `x86_64-pc-linux-gnu`.
|
@llvm/pr-subscribers-tools-llvm-exegesis Author: Rainer Orth (rorth) Changes… hosts Three
It seems like this test only works on little-endian hosts: three sub-tests are already disabled on powerpc and s390x (both big-endian), and the fourth is additionally guarded against big-endian hosts (making the other guards unnecessary). Rather than add big-endian hosts (like sparc) one by one to every sub-test, it seems better to disable the whole test on big-endian. Tested on Full diff: https://github.com/llvm/llvm-project/pull/102755.diff 1 Files Affected:
diff --git a/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp b/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
index 7c23e7b7e9c5a5..b6dfca2da6c625 100644
--- a/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
+++ b/llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp
@@ -24,7 +24,9 @@
namespace llvm {
namespace exegesis {
-#if defined(__linux__) && !defined(__ANDROID__)
+// The tests are only supported on little endian systems.
+#if defined(__linux__) && !defined(__ANDROID__) && \
+ __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
// This needs to be updated anytime a test is added or removed from the test
// suite.
@@ -77,20 +79,12 @@ class SubprocessMemoryTest : public X86TestBase {
// memory calls not working in some cases, so they have been disabled.
// TODO(boomanaiden154): Investigate and fix this issue on PPC.
-#if defined(__powerpc__) || defined(__s390x__)
-TEST_F(SubprocessMemoryTest, DISABLED_OneDefinition) {
-#else
TEST_F(SubprocessMemoryTest, OneDefinition) {
-#endif
testCommon({{"test1", {APInt(8, 0xff), 4096, 0}}}, 0);
checkSharedMemoryDefinition(getSharedMemoryName(0, 0), 4096, {0xff});
}
-#if defined(__powerpc__) || defined(__s390x__)
-TEST_F(SubprocessMemoryTest, DISABLED_MultipleDefinitions) {
-#else
TEST_F(SubprocessMemoryTest, MultipleDefinitions) {
-#endif
testCommon({{"test1", {APInt(8, 0xaa), 4096, 0}},
{"test2", {APInt(8, 0xbb), 4096, 1}},
{"test3", {APInt(8, 0xcc), 4096, 2}}},
@@ -100,11 +94,7 @@ TEST_F(SubprocessMemoryTest, MultipleDefinitions) {
checkSharedMemoryDefinition(getSharedMemoryName(1, 2), 4096, {0xcc});
}
-#if defined(__powerpc__) || defined(__s390x__)
-TEST_F(SubprocessMemoryTest, DISABLED_DefinitionFillsCompletely) {
-#else
TEST_F(SubprocessMemoryTest, DefinitionFillsCompletely) {
-#endif
testCommon({{"test1", {APInt(8, 0xaa), 4096, 0}},
{"test2", {APInt(16, 0xbbbb), 4096, 1}},
{"test3", {APInt(24, 0xcccccc), 4096, 2}}},
@@ -117,12 +107,7 @@ TEST_F(SubprocessMemoryTest, DefinitionFillsCompletely) {
checkSharedMemoryDefinition(getSharedMemoryName(2, 2), 4096, Test3Expected);
}
-// The following test is only supported on little endian systems.
-#if defined(__powerpc__) || defined(__s390x__) || __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-TEST_F(SubprocessMemoryTest, DISABLED_DefinitionEndTruncation) {
-#else
TEST_F(SubprocessMemoryTest, DefinitionEndTruncation) {
-#endif
testCommon({{"test1", {APInt(48, 0xaabbccddeeff), 4096, 0}}}, 3);
std::vector<uint8_t> Test1Expected(512, 0);
// order is reversed since we're assuming a little endian system.
@@ -150,7 +135,8 @@ TEST_F(SubprocessMemoryTest, DefinitionEndTruncation) {
checkSharedMemoryDefinition(getSharedMemoryName(3, 0), 4096, Test1Expected);
}
-#endif // defined(__linux__) && !defined(__ANDROID__)
+#endif // defined(__linux__) && !defined(__ANDROID__) && __BYTE_ORDER__ ==
+ // __ORDER_LITTLE_ENDIAN__
} // namespace exegesis
} // namespace llvm
|
There was a problem hiding this comment.
There is only one test here that should actually be dependent on endianness (the one marked explicitly).
I've never run this test on a big-endian system myself to debug, so I can't say for sure, but I would suspect that there is a different issue (the zero value is really suspicious from your test failures)
Ultimately though, this test is for functionality that is only enabled on x86_64. I'd rather not carve out x86_64 specifically to prevent people missing this on bringup though.
I'm not opposed to this patch, but I'd probably prefer a carve out specific to Sparc pending more investigation.
I see. I'd rather not try this myself since I've just spent 3 days digging through a large and completely unfamilier part of the code base to fix two failing tests.
So would it be ok to move the |
rorth
changed the title
|
/cherry-pick a417083 |
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 10, 2024
…llvm#102755) Three `llvm-exegesis` tests ``` LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/DefinitionFillsCompletely LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/MultipleDefinitions LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/OneDefinition ``` `FAIL` on Linux/sparc64 like ``` llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp:68: Failure Expected equality of these values: SharedMemoryMapping[I] Which is: '\0' ExpectedValue[I] Which is: '\xAA' (170) ``` It seems like this test only works on little-endian hosts: three sub-tests are already disabled on powerpc and s390x (both big-endian), and the fourth is additionally guarded against big-endian hosts (making the other guards unnecessary). However, since it's not been analyzed if this is really an endianess issue, this patch disables the whole test on powerpc and s390x as before adding sparc to the mix. Tested on `sparc64-unknown-linux-gnu` and `x86_64-pc-linux-gnu`. (cherry picked from commit a417083)
|
/pull-request #102771 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/1656 Here is the relevant piece of the build log for the reference: |
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 12, 2024
…llvm#102755) Three `llvm-exegesis` tests ``` LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/DefinitionFillsCompletely LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/MultipleDefinitions LLVM-Unit :: tools/llvm-exegesis/./LLVMExegesisTests/SubprocessMemoryTest/OneDefinition ``` `FAIL` on Linux/sparc64 like ``` llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp:68: Failure Expected equality of these values: SharedMemoryMapping[I] Which is: '\0' ExpectedValue[I] Which is: '\xAA' (170) ``` It seems like this test only works on little-endian hosts: three sub-tests are already disabled on powerpc and s390x (both big-endian), and the fourth is additionally guarded against big-endian hosts (making the other guards unnecessary). However, since it's not been analyzed if this is really an endianess issue, this patch disables the whole test on powerpc and s390x as before adding sparc to the mix. Tested on `sparc64-unknown-linux-gnu` and `x86_64-pc-linux-gnu`. (cherry picked from commit a417083)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Three
llvm-exegesistestsFAILon Linux/sparc64 likeIt seems like this test only works on little-endian hosts: three sub-tests are already disabled on powerpc and s390x (both big-endian), and the fourth is additionally guarded against big-endian hosts (making the other guards unnecessary).
However, since it's not been analyzed if this is really an endianess issue, this patch disables the whole test on powerpc and s390x as before adding sparc to the mix.
Tested on
sparc64-unknown-linux-gnuandx86_64-pc-linux-gnu.