-
Notifications
You must be signed in to change notification settings - Fork 90
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
Export symbol from DLL in test_jit #2861
Conversation
This build is not recommended to merge 🔴 |
🔴bert_large_uncased_fp16: FAILED: MIGraphX is not within tolerance - check verbose output |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2861 +/- ##
========================================
Coverage 91.97% 91.97%
========================================
Files 489 489
Lines 19390 19390
========================================
Hits 17833 17833
Misses 1557 1557 ☔ View full report in Codecov by Sentry. |
test/jit.cpp
Outdated
// NOLINTNEXTLINE | ||
const std::string_view add_42_src = R"migraphx( | ||
const std::string_view add_42_src = EXPORT_SYMBOL R"migraphx( |
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.
Instead, I would add a -DEXPORT=...
define when compiling the test(they all use the compile_function
so can be added there. And then in the string you can just do EXPORT extern
.
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.
Done
test/jit.cpp
Outdated
{ | ||
migraphx::src_compiler compiler; | ||
compiler.flags.emplace_back("-std=c++14"); | ||
#ifndef _WIN32 | ||
compiler.flags.emplace_back("-fPIC"); | ||
compiler.flags.emplace_back("-DEXPORT=\"\""); | ||
#else | ||
compiler.flags.emplace_back("-DEXPORT=\"__declspec(dllexport)\""); |
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 dont think quotes is needed for these flags.
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.
It fails execution on Linux if ""
is missing.
59/302 Test #36: test_jit ...........................................................Subprocess aborted***Exception: 0.22 sec
[ RUN ] simple_run
<command-line>: error: expected unqualified-id before numeric constant
main.cpp:2:1: note: in expansion of macro ‘EXPORT’
2 | EXPORT extern "C" int add(int x)
| ^~~~~~
terminate called after throwing an instance of 'migraphx::version_1::exception'
what(): ../../src/process.cpp:358: check_exec: Command cd /tmp/migraphx-compile-31108-7f4a714a0b80-8fa304c59fe-cTPCBnqDd3c5VKGi; /usr/bin/c++ -std=c++14 -fPIC -DEXPORT -shared -I. main.cpp -o libsimple.so exited with status 1
On Windows, the symbol must be exported from the DLL for the test to see the function while dynamically loading the library.