Skip to content
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

fix the compilation for power pc #288

Closed
wants to merge 1 commit into from

Conversation

eddy16112
Copy link

This PR is to fix the following error on power pc

In file included from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/util/../external/marray/include/expression.hpp:4,
                 from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/util/../external/marray/include/marray_base.hpp:509,
                 from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/util/../external/marray/include/varray_base.hpp:4,
                 from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/util/../external/marray/include/varray_view.hpp:4,
                 from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/util/../external/marray/include/varray.hpp:4,
                 from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/util/basic_types.h:38,
                 from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/util/configs.h:4,
                 from /autofs/nccs-svm1_home1/wwu12/install/TBLIS/include/tblis/tblis.h:4,
                 from cunumeric/matrix/contract.cc:21:
/autofs/nccs-svm1_sw/summit/gcc/9.3.0-2/lib/gcc/powerpc64le-unknown-linux-gnu/9.3.0/include/x86intrin.h:32:2: error: #error "Please read comment above.  Use -DNO_WARN_X86_INTRINSICS to disable this error."

@magnatelee
Copy link
Contributor

LGTM, but I want @manopapad to take a look at this as well.

Copy link
Contributor

@manopapad manopapad left a comment

Choose a reason for hiding this comment

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

LGTM

@lightsighter
Copy link
Contributor

Shouldn't we really ping the TBLIS library developers to make sure that they are masking their intrinsic headers correctly? Do we know if they even support PowerPC vector intrinsics?

@eddy16112
Copy link
Author

eddy16112 commented Apr 20, 2022

Let me create a issue on their github repo to see what do they say about this error.
devinamatthews/tblis#45

@manopapad manopapad changed the base branch from branch-22.05 to branch-22.10 August 23, 2022 20:39
@manopapad
Copy link
Contributor

The ppc build for TBLIS has not yet materialized, so this is still necessary. However, the make-based build workflow has been removed since this PR was submitted, so we need to port this to the cmake-based build.

@trxcllnt can we specifically pass -DNO_WARN_X86_INTRINSICS through to the tblis compilation when on powerpc? I have been manually passing it through CFLAGS/CXXFLAGS on the relevant platforms that we frequent. https://github.com/nv-legate/quickstart/blob/branch-22.12/common.sh#L53-L58

@manopapad manopapad closed this Nov 23, 2022
@martin-frbg
Copy link

I'm now trying to build cunumeric on POWER9 myself, hitting the same issue and wondering what happened to this PR (and topic in general) ? tblis still has not changed anything, and even trying to feed a local build of a modified tblis to install.py does not appear to work. install.py --with-tblis <location> is documented in install.py, but the build still downloads a fresh copy of tblis everytime instead of using the (patched) sources from the provided location. (Reapplying the patch to the downloaded copy in _skbuild and rerunning install.py with the --no-clean option also does not help - still overwritten with a fresh copy...) This is on a shared machine, so I cannot easily edit the system headers

@manopapad
Copy link
Contributor

I have opened a tblis PR that fixes the build on ARM. I believe it also fixes the build on PowerPC, but I don't have access to a PowerPC machine to test it myself. You can try telling cuNumeric to use the PR branch directly during compilation and see if that fixes it for you:

diff --git a/cmake/thirdparty/get_tblis.cmake b/cmake/thirdparty/get_tblis.cmake
index b02afbd7..d7f1d289 100644
--- a/cmake/thirdparty/get_tblis.cmake
+++ b/cmake/thirdparty/get_tblis.cmake
@@ -171,11 +171,11 @@ function(find_or_configure_tblis)
 endfunction()

 if(NOT DEFINED cunumeric_TBLIS_BRANCH)
-  set(cunumeric_TBLIS_BRANCH master)
+  set(cunumeric_TBLIS_BRANCH arm-build)
 endif()

 if(NOT DEFINED cunumeric_TBLIS_REPOSITORY)
-  set(cunumeric_TBLIS_REPOSITORY https://github.com/devinamatthews/tblis.git)
+  set(cunumeric_TBLIS_REPOSITORY https://github.com/manopapad/tblis.git)
 endif()

 find_or_configure_tblis(VERSION          1.2.0

@martin-frbg
Copy link

Thank you - I can confirm now that this fixes the build on PowerPC as well (tested on host cfarm120 in the gcc compile farm, a 24core/192threads POWER10 machine running AlmaLinux9.3)

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.

None yet

5 participants