-
Notifications
You must be signed in to change notification settings - Fork 121
Refactor Init to delay Library Load #379
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
base: master
Are you sure you want to change the base?
Conversation
c8b0eb3 to
3e550da
Compare
20c2abe to
f35ffc2
Compare
|
Abi issues are invalid so please ignore. |
7d3c02b to
e3858a5
Compare
|
Because some of the tests are changed due to the refactor, the tests that are failing in the n-1 are expected. they are the changed tests. |
- Delay to load of the L0 Driver Libraries until the user has directly requested the library to be loaded thru the flags. - Remove the "pretest" of driver support and instead assume driver support based on naming and registry categorization. Speeds up loader init. - Moved init of drivers until zInitDrivers or zeInit - Added ULTs to verify the new init_driver functionality Signed-off-by: Neil R. Spruit <[email protected]>
Signed-off-by: Neil R. Spruit <[email protected]>
Signed-off-by: Neil R. Spruit <[email protected]>
Signed-off-by: Neil R. Spruit <[email protected]>
e3858a5 to
cee5877
Compare
| -D CMAKE_C_COMPILER_LAUNCHER=ccache \ | ||
| -D CMAKE_CXX_COMPILER_LAUNCHER=ccache \ | ||
| -D CMAKE_BUILD_TYPE=Release \ | ||
| -D BUILD_L0_LOADER_TESTS=1 \ |
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 may not be required since BUILD_L0_LOADER_TESTS is enabled for static loader build
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.
Because of the fake drivers I have created this indicates the creation of those libraries as part of that build option. so, even though we don't use the "tests" in the dynamic build folder, we will use the test libraries in the lib.
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.
Got it. Then I guess the same thing can be added to buil-quick-static-n-1 aswell.
So that for static n-1 testing for future PR's (which will have driver init tests as part of n-1 static build), fake libs gets generated in dynamic_loader folder to use it for driver init tests
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.
Other than this, looks good to me!
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.
fixed in latest, thanks!
| desc.pNext = nullptr; | ||
| putenv_safe( const_cast<char *>( "ZEL_TEST_MISSING_API=zeInitDrivers" ) ); | ||
| EXPECT_EQ(ZE_RESULT_ERROR_UNINITIALIZED, zeInitDrivers(&pInitDriversCount, nullptr, &desc)); | ||
| EXPECT_EQ(ZE_RESULT_ERROR_UNSUPPORTED_FEATURE, zeInitDrivers(&pInitDriversCount, nullptr, &desc)); |
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.
minor: test name could also be updated
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.
Good point, I will update that
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.
fixed in latest
| EXPECT_EQ(ZE_RESULT_ERROR_UNSUPPORTED_FEATURE, result); | ||
| EXPECT_EQ(0, tracingData.getZerPrologueCallCount("zerGetLastErrorDescription")); | ||
| EXPECT_EQ(0, tracingData.getZerEpilogueCallCount("zerGetLastErrorDescription")); | ||
| // ZER callbacks should still be called in the tracing layer even if the driver ends up not supporting ZER APIs |
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.
minor: test name can be updated here aswell
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.
Good point, I will rewrite the test name.
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.
fixed in latest
Signed-off-by: Neil R. Spruit <[email protected]>
Signed-off-by: Neil R. Spruit <[email protected]>
requested the library to be loaded thru the flags.
support based on naming and registry categorization. Speeds up loader
init.