-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[offload][lit] Fix compilation of two offload tests #169399
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
Signed-off-by: Nick Sarnie <[email protected]>
|
@llvm/pr-subscribers-offload Author: Nick Sarnie (sarnex) ChangesThese are C tests, not C++, so no function parameters means unspecified number of parameters, not These compile fine on the current tested offload targets because an error is only thrown if the calling convention doesn't support variadic arguments, which they happen to. When compiling this test for other targets that do not support variadic arguments, we get an error, which does not seem intentional. Just add Full diff: https://github.com/llvm/llvm-project/pull/169399.diff 2 Files Affected:
diff --git a/offload/test/offloading/shared_lib_fp_mapping.c b/offload/test/offloading/shared_lib_fp_mapping.c
index c6203443eee18..0e1fed4d0baf8 100644
--- a/offload/test/offloading/shared_lib_fp_mapping.c
+++ b/offload/test/offloading/shared_lib_fp_mapping.c
@@ -7,7 +7,7 @@
#include <stdio.h>
-extern int func(); // Provided in liba.so, returns 42
+extern int func(void); // Provided in liba.so, returns 42
typedef int (*fp_t)();
int main() {
diff --git a/offload/test/offloading/static_linking.c b/offload/test/offloading/static_linking.c
index 7be95a10ffcd6..273109e10c09e 100644
--- a/offload/test/offloading/static_linking.c
+++ b/offload/test/offloading/static_linking.c
@@ -14,7 +14,7 @@ int foo() {
}
#else
#include <stdio.h>
-int foo();
+int foo(void);
int main() {
int x = foo();
|
|
|
||
| extern int func(); // Provided in liba.so, returns 42 | ||
| extern int func(void); // Provided in liba.so, returns 42 | ||
| typedef int (*fp_t)(); |
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.
Same for the typedef?
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.
I was using SPIR-V to repro the issue and it compiled successfully with just the func change.
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.
I think you need the void in the typedef too, otherwise it's a type mismatch. Most compilers will accept it. Clang just reports a warning.
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.
Added in latest commit, thanks!
Signed-off-by: Nick Sarnie <[email protected]>
kevinsala
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.
LGTM
These are C tests, not C++, so no function parameters means unspecified number of parameters, not
void.These compile fine on the current tested offload targets because an error is only thrown if the calling convention doesn't support variadic arguments, which they happen to.
When compiling this test for other targets that do not support variadic arguments, we get an error, which does not seem intentional.
Just add
voidto the parameter list.