-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc] Make errno asserts noop on gpu targets #166606
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
Conversation
fc9c2b3 to
8dba906
Compare
8dba906 to
8e7fabe
Compare
|
@llvm/pr-subscribers-libc Author: Marcell Leleszi (mleleszi) ChangesThis patch defines errno unit and integration test asserts as noop on GPU targets. Checking for errnos is tests has caused build breakages in previous patches. Full diff: https://github.com/llvm/llvm-project/pull/166606.diff 2 Files Affected:
diff --git a/libc/test/IntegrationTest/test.h b/libc/test/IntegrationTest/test.h
index 4a03f7aa6318b..4fc0566316a40 100644
--- a/libc/test/IntegrationTest/test.h
+++ b/libc/test/IntegrationTest/test.h
@@ -68,9 +68,15 @@
////////////////////////////////////////////////////////////////////////////////
// Errno checks.
+#ifdef LIBC_TARGET_ARCH_IS_GPU
+#define ASSERT_ERRNO_EQ(VAL)
+#define ASSERT_ERRNO_SUCCESS()
+#define ASSERT_ERRNO_FAILURE()
+#else
#define ASSERT_ERRNO_EQ(VAL) ASSERT_EQ(VAL, static_cast<int>(errno))
#define ASSERT_ERRNO_SUCCESS() ASSERT_EQ(0, static_cast<int>(errno))
#define ASSERT_ERRNO_FAILURE() ASSERT_NE(0, static_cast<int>(errno))
+#endif
// Integration tests are compiled with -ffreestanding which stops treating
// the main function as a non-overloadable special function. Hence, we use a
diff --git a/libc/test/UnitTest/ErrnoCheckingTest.h b/libc/test/UnitTest/ErrnoCheckingTest.h
index 5b1bc9441d830..2aa2ce70bfbb5 100644
--- a/libc/test/UnitTest/ErrnoCheckingTest.h
+++ b/libc/test/UnitTest/ErrnoCheckingTest.h
@@ -16,6 +16,11 @@
// Define macro to validate the value stored in the errno and restore it
// to zero.
+#ifdef LIBC_TARGET_ARCH_IS_GPU
+#define ASSERT_ERRNO_EQ(VAL)
+#define ASSERT_ERRNO_SUCCESS()
+#define ASSERT_ERRNO_FAILURE()
+#else
#define ASSERT_ERRNO_EQ(VAL) \
do { \
ASSERT_EQ(VAL, static_cast<int>(libc_errno)); \
@@ -27,6 +32,7 @@
ASSERT_NE(0, static_cast<int>(libc_errno)); \
libc_errno = 0; \
} while (0)
+#endif
namespace LIBC_NAMESPACE_DECL {
namespace testing {
|
jhuber6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably fine as a quick work around but long term it might be better to manage this with a config option to totally disable errno.
|
Is this PR supposed to be the fix (I saw another PR)? I can run it locally to see if it works. |
|
Both are needed, so let’s wait for this one to get merged so i can rebase and you can test the other one. I’ll ping you when you can test it. Thanks! |
|
@lntue can you merge this one please |
|
please make sure this fix (#166601) is included in when you rebase. |
This patch defines errno unit and integration test asserts as noop on GPU targets. Checking for errnos is tests has caused build breakages in previous patches.