-
Notifications
You must be signed in to change notification settings - Fork 133
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
USE_ARCH flag work for Heterogenous Galois too #92
Comments
I did not understand this. Why do we need this? |
It is a little unexpected that https://github.com/IntelligentSoftwareSystems/Galois/blob/master/CMakeLists.txt#L69 |
Yah, it's an unexpected behavior issue. I'm not aware of anything that makes this urgent. The current behavior is just weird. |
What is USE_ARCH expected to do? I will be renaming ENABLE_HETERO_GALOIS to GALOIS_ENABLE_GPU (and merging it with USE_GPU). That flag will enable single/distributed GPU code for lonestar (analytics/mining/scientific). |
USE_ARCH sets the appropriate |
I don't see USE_ARCH being used anywhere currently. It was being used for KNL if I remember right. But where is that code in our cmake files?
I will drop those lines. They are not related/required. |
It's in https://github.com/IntelligentSoftwareSystems/Galois/blob/master/cmake/Modules/CheckArchFlags.cmake which defined variables that then get pulled in here: https://github.com/IntelligentSoftwareSystems/Galois/blob/master/CMakeLists.txt#L132. |
My best guess is that the if statement at https://github.com/IntelligentSoftwareSystems/Galois/blob/master/CMakeLists.txt#L69 was added to work around some kind of compiler error for the heterogeneous case. @ddn0, you added that, do you remember why it's there? |
Thanks, Ian. This is not related to GALOIS_ENABLE_GPU flag, so waiting for Donald's reply to see if he added those lines. |
I added that line to follow the original logic: which was added in this commit: That's where my context ends :D Unrelated to this issue, I noticed there are still quite a few warnings when compiling gluon (unused parameters) and with gcc 8.4/clang 9, I actually get an internal compiler error:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 13a2e9376..91ed1a9e8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -66,9 +66,9 @@ if(NUM_TEST_THREADS LESS_EQUAL 0)
set(NUM_TEST_THREADS 1)
endif()
-if(ENABLE_HETERO_GALOIS)
- set(USE_ARCH none)
-endif()
+# if(ENABLE_HETERO_GALOIS)
+# set(USE_ARCH none)
+# endif()
###### Configure (users don't need to go beyond here) ######
@@ -93,7 +93,7 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
message(FATAL_ERROR "gcc must be version 7 or higher. Found ${CMAKE_CXX_COMPILER_VERSION}.")
endif()
- add_compile_options("$<$<COMPILE_LANGUAGE:CXX>:-Wall;-Wextra;-Werror>")
+ add_compile_options("$<$<COMPILE_LANGUAGE:CXX>:-Wall;-Wextra;-Werror;-w>")
if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9)
# Avoid warnings from openmpi |
Thanks, Donald. This was my commit but those lines were there from a commit before that. It's pretty old. I don't think it's needed. |
Are these errors related to commenting out those lines? |
There are two issues:
I'll file issues for both. |
Thanks! |
I made those flags independent in PR 199. |
Not a blocker, but we should make the USE_ARCH flag work everywhere. Even for Heterogeneous Galois, we can still set the architecture flag for all the CPU code generated.
Originally posted by @insertinterestingnamehere in #88
The text was updated successfully, but these errors were encountered: