Skip to content

Conversation

@JayFoxRox
Copy link

(I'll add a description later)

Part of CHIP-SPV/chipStar#602

@JayFoxRox JayFoxRox mentioned this pull request Aug 27, 2023
17 tasks
set(CMAKE_CXX_COMPILER "${HIP_PATH}/bin/hipcc.bin")
set(CMAKE_C_COMPILER "${HIP_PATH}/bin/hipcc.bin")
set(CMAKE_CXX_COMPILER "${HIP_PATH}/bin/hipcc")
set(CMAKE_C_COMPILER "${HIP_PATH}/bin/hipcc")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should make it possible to use the perl wrapper

set(CMAKE_C_COMPILER "${HIP_PATH}/bin/hipcc.bin")
set(CMAKE_CXX_COMPILER "${HIP_PATH}/bin/hipcc")
set(CMAKE_C_COMPILER "${HIP_PATH}/bin/hipcc")
set(HIPCONFIG_EXECUTABLE "${HIP_PATH}/bin/hipconfig.bin")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be changed to hipconfig so it uses the perl wrapper

message("Standalone Catch test: ${exec_name} ${src}")
add_executable(${exec_name} EXCLUDE_FROM_ALL ${src} $<TARGET_OBJECTS:Main_Object_Standalone> $<TARGET_OBJECTS:KERNELS>)
target_link_libraries(${exec_name} PRIVATE stdc++fs -pthread )
target_link_libraries(${exec_name} PRIVATE -pthread )
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdc++fs is spammed all over the HIP codebases, despite often requiring c++17 or newer and with bad conditions.

These changes might break it for some users, but rather than adding it manually, we should be using CMake to add filesystem support, OR, require c++17 through CMake and then fail.

#elif defined(__APPLE__) || defined(__MACOSX)
#define HT_WIN 0
#define HT_LINUX 0
#define HT_MACOS 1
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea where these new flags are used.
The existing code quality here is pretty horrible.

#ifdef __linux__
#include <sys/sysinfo.h>
#elif defined(__APPLE__) || defined(__MACOSX)
// No-op
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be different

# skipped due to os related code in tests
# need to work on them when all the tests are enabled
if(UNIX)
if(UNIX AND NOT APPLE)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests might be portable. I didn't check yet.


*total = (vmstat.wire_count + vmstat.active_count + vmstat.inactive_count + vmstat.free_count + Pages occupied by compressor + Pages speculative) * pageSize;
*free = vmstat.free_count * pageSize;
#endif
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be activated. Consider it WIP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants