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

PETSc is broken on ARM64 #16432

Closed
Tracked by #17009
mwoehlke-kitware opened this issue Jan 25, 2022 · 7 comments
Closed
Tracked by #17009

PETSc is broken on ARM64 #16432

mwoehlke-kitware opened this issue Jan 25, 2022 · 7 comments
Assignees
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters priority: medium type: feature request

Comments

@mwoehlke-kitware
Copy link
Contributor

PETSc is explicitly using XMM intrinsics, which are not available on ARM, leading to build errors.

The following patch can "get past" the problems, but it's unclear how this file was generated, or whether these changes are "correct". They also probably should be conditional to ARM builds.

diff --git a/tools/workspace/petsc/stubs/petscconf.h b/tools/workspace/petsc/stubs/petscconf.h
index e255b7c11..a09bdf5dc 100644
--- a/tools/workspace/petsc/stubs/petscconf.h
+++ b/tools/workspace/petsc/stubs/petscconf.h
@@ -117,7 +117,7 @@
 #define PETSC_HAVE_USLEEP 1
 #define PETSC_HAVE_VA_COPY 1
 #define PETSC_HAVE_VSNPRINTF 1
-#define PETSC_HAVE_XMMINTRIN_H 1
+//#define PETSC_HAVE_XMMINTRIN_H 1
 #define PETSC_IS_COLORING_MAX USHRT_MAX
 #define PETSC_IS_COLORING_VALUE_TYPE short  // NOLINT
 #define PETSC_IS_COLORING_VALUE_TYPE_F integer2
@@ -127,12 +127,12 @@
 #define PETSC_MEMALIGN 8
 #define PETSC_MPICC_SHOW "Unavailable"
 #define PETSC_MPIU_IS_COLORING_VALUE_TYPE MPI_UNSIGNED_SHORT
-#define PETSC_PREFETCH_HINT_NTA _MM_HINT_NTA
-#define PETSC_PREFETCH_HINT_T0 _MM_HINT_T0
-#define PETSC_PREFETCH_HINT_T1 _MM_HINT_T1
-#define PETSC_PREFETCH_HINT_T2 _MM_HINT_T2
+#define PETSC_PREFETCH_HINT_NTA 0 //_MM_HINT_NTA
+#define PETSC_PREFETCH_HINT_T0 0 //_MM_HINT_T0
+#define PETSC_PREFETCH_HINT_T1 0 //_MM_HINT_T1
+#define PETSC_PREFETCH_HINT_T2 0 //_MM_HINT_T2
 #define PETSC_PYTHON_EXE "NO_PETSC_PYTHON_EXE"
-#define PETSC_Prefetch(a, b, c) _mm_prefetch((const char*)(a), (c))
+#define PETSC_Prefetch(a, b, c) (void)((a), (c)) //_mm_prefetch((const char*)(a), (c))
 #define PETSC_REPLACE_DIR_SEPARATOR '\\'
 #define PETSC_SIGNAL_CAST
 #define PETSC_SIZEOF_ENUM 4

Relates to #16380, #13514.

@jwnimmer-tri jwnimmer-tri added component: build system Bazel, CMake, dependencies, memory checkers, linters unused team: kitware type: feature request labels Jan 25, 2022
@jwnimmer-tri
Copy link
Collaborator

(Blocked by #16380.)

@jwnimmer-tri
Copy link
Collaborator

jwnimmer-tri commented Apr 25, 2022

... it's unclear how this file was generated ...

The comment at the top of the file explains:

This file is a lightly-edited version of what comes out of upstream's configure script...

For Drake's M1 builds, the first step on this issue should be to manually run the upstream configure on an M1 (in ARM mode), and compare that to the existing checked-in file.

It's possible that we'll need to keep two copies of the stub file (and switch between them), or maybe we could keep a single stub file but add in some ifdef's to sense the platform and adjust accordingly.

@mwoehlke-kitware
Copy link
Contributor Author

Here's the (mostly¹) complete diff of the generated-on-M1 petscconf.h vs. what's in Drake:

diff --git a/tools/workspace/petsc/stubs/petscconf.h b/tools/workspace/petsc/stubs/petscconf.h
index e255b7c11f..66870b3f88 100644
--- a/tools/workspace/petsc/stubs/petscconf.h
+++ b/tools/workspace/petsc/stubs/petscconf.h
@@ -14,6 +14,8 @@
 #define PETSC_ARCH "NO_PETSC_ARCH"
 #define PETSC_ATTRIBUTEALIGNED(size) __attribute((aligned(size)))
 #define PETSC_Alignx(a, b)
+#define PETSC_BLASLAPACK_SDOT_RETURNS_DOUBLE 1
+#define PETSC_BLASLAPACK_SNRM2_RETURNS_DOUBLE 1
 #define PETSC_BLASLAPACK_UNDERSCORE 1
 #define PETSC_CLANGUAGE_C 1
 #define PETSC_CXX_INLINE inline
@@ -26,6 +28,9 @@
 #define PETSC_DEPRECATED_TYPEDEF(why) __attribute((deprecated))
 #define PETSC_DIR "NO_PETSC_DIR"
 #define PETSC_DIR_SEPARATOR '/'
+#define PETSC_DO_NOT_SWAP_CHILD_FOR_DEBUGGER 1
+#define PETSC_FORTRAN_CHARLEN_T size_t
+#define PETSC_FORTRAN_TYPE_INITIALIZE  = -2
 #define PETSC_FUNCTION_NAME_C __func__
 #define PETSC_FUNCTION_NAME_CXX __func__
 #define PETSC_HAVE_ACCESS 1
@@ -35,7 +40,9 @@
 #define PETSC_HAVE_BZERO 1
 #define PETSC_HAVE_C99_COMPLEX 1
 #define PETSC_HAVE_CLOCK 1
+#define PETSC_HAVE_CLOSURE 1
 #define PETSC_HAVE_CXX 1
+#define PETSC_HAVE_CXXABI_H 1
 #define PETSC_HAVE_CXX_COMPLEX 1
 #define PETSC_HAVE_CXX_COMPLEX_FIX 1
 #define PETSC_HAVE_CXX_DIALECT_CXX03 1
@@ -56,12 +63,18 @@
 #define PETSC_HAVE_FENV_H 1
 #define PETSC_HAVE_FLOAT_H 1
 #define PETSC_HAVE_FORK 1
+#define PETSC_HAVE_FORTRAN 1
+#define PETSC_HAVE_FORTRAN_FLUSH 1
+#define PETSC_HAVE_FORTRAN_GET_COMMAND_ARGUMENT 1
+#define PETSC_HAVE_FORTRAN_TYPE_STAR 1
+#define PETSC_HAVE_FORTRAN_UNDERSCORE 1
 #define PETSC_HAVE_GETCWD 1
 #define PETSC_HAVE_GETDOMAINNAME 1
 #define PETSC_HAVE_GETHOSTBYNAME 1
 #define PETSC_HAVE_GETHOSTNAME 1
 #define PETSC_HAVE_GETPAGESIZE 1
 #define PETSC_HAVE_GETRUSAGE 1
+#define PETSC_HAVE_GETWD 1
 #define PETSC_HAVE_IMMINTRIN_H 1
 #define PETSC_HAVE_INTTYPES_H 1
 #define PETSC_HAVE_ISINF 1
@@ -70,6 +83,7 @@
 #define PETSC_HAVE_LGAMMA 1
 #define PETSC_HAVE_LOG2 1
 #define PETSC_HAVE_LSEEK 1
+#define PETSC_HAVE_MACHINE_ENDIAN_H 1
 #define PETSC_HAVE_MEMMOVE 1
 #define PETSC_HAVE_MMAP 1
 #define PETSC_HAVE_MPIUNI 1
@@ -79,14 +93,14 @@
 #define PETSC_HAVE_NANOSLEEP 1
 #define PETSC_HAVE_NETDB_H 1
 #define PETSC_HAVE_NETINET_IN_H 1
-#define PETSC_HAVE_PACKAGES ":blaslapack:mathlib:mpi:regex:"
+#define PETSC_HAVE_PACKAGES ":blaslapack:mathlib:mpi:pthread:regex:sowing:"
 #define PETSC_HAVE_POPEN 1
+#define PETSC_HAVE_PTHREAD 1
 #define PETSC_HAVE_PTHREAD_H 1
 #define PETSC_HAVE_PWD_H 1
 #define PETSC_HAVE_RAND 1
 #define PETSC_HAVE_READLINK 1
 #define PETSC_HAVE_REALPATH 1
-#define PETSC_HAVE_REAL___FLOAT128 1
 #define PETSC_HAVE_REGEX 1
 #define PETSC_HAVE_RTLD_GLOBAL 1
 #define PETSC_HAVE_RTLD_LAZY 1
@@ -95,21 +109,23 @@
 #define PETSC_HAVE_SETJMP_H 1
 #define PETSC_HAVE_SLEEP 1
 #define PETSC_HAVE_SNPRINTF 1
+#define PETSC_HAVE_SOCKET 1
+#define PETSC_HAVE_SOWING 1
+#define PETSC_HAVE_SO_REUSEADDR 1
 #define PETSC_HAVE_STDINT_H 1
 #define PETSC_HAVE_STRCASECMP 1
 #define PETSC_HAVE_STRINGS_H 1
 #define PETSC_HAVE_STRUCT_SIGACTION 1
-#define PETSC_HAVE_SYSINFO 1
 #define PETSC_HAVE_SYS_PARAM_H 1
 #define PETSC_HAVE_SYS_RESOURCE_H 1
 #define PETSC_HAVE_SYS_SOCKET_H 1
+#define PETSC_HAVE_SYS_SYSCTL_H 1
 #define PETSC_HAVE_SYS_TIMES_H 1
 #define PETSC_HAVE_SYS_TIME_H 1
 #define PETSC_HAVE_SYS_TYPES_H 1
 #define PETSC_HAVE_SYS_UTSNAME_H 1
 #define PETSC_HAVE_SYS_WAIT_H 1
 #define PETSC_HAVE_TGAMMA 1
-#define PETSC_HAVE_THREADSAFETY 1
 #define PETSC_HAVE_TIME 1
 #define PETSC_HAVE_TIME_H 1
 #define PETSC_HAVE_UNAME 1
@@ -117,23 +133,23 @@
 #define PETSC_HAVE_USLEEP 1
 #define PETSC_HAVE_VA_COPY 1
 #define PETSC_HAVE_VSNPRINTF 1
-#define PETSC_HAVE_XMMINTRIN_H 1
 #define PETSC_IS_COLORING_MAX USHRT_MAX
 #define PETSC_IS_COLORING_VALUE_TYPE short  // NOLINT
 #define PETSC_IS_COLORING_VALUE_TYPE_F integer2
-#define PETSC_LEVEL1_DCACHE_LINESIZE 64
-#define PETSC_LIB_DIR "NO_PETSC_LIB_DIR"
-#define PETSC_MAX_PATH_LEN 4096
-#define PETSC_MEMALIGN 8
+#define PETSC_LEVEL1_DCACHE_LINESIZE 32
+#define PETSC_LIB_DIR "/Users/matthew.woehlke/petsc/arch-darwin-c-debug/lib"
+#define PETSC_MAX_PATH_LEN 1024
+#define PETSC_MEMALIGN 16
 #define PETSC_MPICC_SHOW "Unavailable"
 #define PETSC_MPIU_IS_COLORING_VALUE_TYPE MPI_UNSIGNED_SHORT
-#define PETSC_PREFETCH_HINT_NTA _MM_HINT_NTA
-#define PETSC_PREFETCH_HINT_T0 _MM_HINT_T0
-#define PETSC_PREFETCH_HINT_T1 _MM_HINT_T1
-#define PETSC_PREFETCH_HINT_T2 _MM_HINT_T2
+#define PETSC_PREFETCH_HINT_NTA 0
+#define PETSC_PREFETCH_HINT_T0 3
+#define PETSC_PREFETCH_HINT_T1 2
+#define PETSC_PREFETCH_HINT_T2 1
 #define PETSC_PYTHON_EXE "NO_PETSC_PYTHON_EXE"
-#define PETSC_Prefetch(a, b, c) _mm_prefetch((const char*)(a), (c))
+#define PETSC_Prefetch(a,b,c) __builtin_prefetch((a),(b),(c))
 #define PETSC_REPLACE_DIR_SEPARATOR '\\'
+#define PETSC_RTLD_DEFAULT 1
 #define PETSC_SIGNAL_CAST  
 #define PETSC_SIZEOF_ENUM 4
 #define PETSC_SIZEOF_INT 4
@@ -145,17 +161,24 @@
 #define PETSC_SLSUFFIX "so"
 #define PETSC_UINTPTR_T uintptr_t
 #define PETSC_UNUSED __attribute((unused))
+#define PETSC_USE_AVX512_KERNELS 1
 #define PETSC_USE_BACKWARD_LOOP 1
 #define PETSC_USE_CTABLE 1
+#define PETSC_USE_DEBUG 1
+#define PETSC_USE_DEBUGGER "lldb"
 #define PETSC_USE_INFO 1
 #define PETSC_USE_ISATTY 1
-#define PETSC_USE_MALLOC_COALESCED 1
-#define PETSC_USE_PROC_FOR_SIZE 1
+#define PETSC_USE_LOG 1
 #define PETSC_USE_REAL_DOUBLE 1
+#define PETSC_USE_SHARED_LIBRARIES 1
 #define PETSC_USE_SINGLE_LIBRARY 1
+#define PETSC_USE_SOCKET_VIEWER 1
 #define PETSC_USE_VISIBILITY_C 1
 #define PETSC_USE_VISIBILITY_CXX 1
 #define PETSC_USING_64BIT_PTR 1
+#define PETSC_USING_DARWIN 1
+#define PETSC_USING_F2003 1
+#define PETSC_USING_F90FREEFORM 1
 #define PETSC_VERSION_BRANCH_GIT "NO_PETSC_VERSION_BRANCH_GIT"
 #define PETSC_VERSION_DATE_GIT "NO_PETSC_VERSION_DATE_GIT"
 #define PETSC_VERSION_GIT "NO_PETSC_VERSION_GIT"

The most critical bits are likely the same as in the original issue description. However, it might not be a bad idea to redo this exercise on all major platforms and create a different petscconf.h for each.

(¹ I removed some irrelevant stuff, such as whitespace-only diffs and build identifier strings that Drake has replaced with placeholders.)

@jwnimmer-tri
Copy link
Collaborator

Some of the changes might come from a newer PETSc version today, versus when I originally ran it. Running it again on Ubuntu 20 x86_64 to compare would be helpful.

Some other of the changes are probably not interesting and we can discard them (e.g., PETSC_USE_LOG).

Some other of the changes are unwanted (e.g., PETSC_USE_SHARED_LIBRARIES).

The similarities of the two confs outweigh the differences. My goal is to use one conffile with preprocessor guards, if we can.

For example:

#ifdef __MMX__
# define PETSC_PREFETCH_HINT_NTA _MM_HINT_NTA
# define PETSC_PREFETCH_HINT_T0 _MM_HINT_T0
# define PETSC_PREFETCH_HINT_T1 _MM_HINT_T1
# define PETSC_PREFETCH_HINT_T2 _MM_HINT_T2
#else
# define PETSC_PREFETCH_HINT_NTA 0
# define PETSC_PREFETCH_HINT_T0 3
# define PETSC_PREFETCH_HINT_T1 2
# define PETSC_PREFETCH_HINT_T2 1
#endif

@mwoehlke-kitware
Copy link
Contributor Author

Some of the changes might come from a newer PETSc version today, versus when I originally ran it.

Could be. I was using v3.16.5 which AFAICT is what's currently in Drake.

The similarities of the two confs outweigh the differences.

I was wondering in particular about changes such as PETSC_USING_F2003, PETSC_MEMALIGN, PETSC_LEVEL1_DCACHE_LINESIZE, PETSC_MAX_PATH_LEN and so forth. Some of those might affect performance. Some like PETSC_HAVE_REAL___FLOAT128 and PETSC_HAVE_THREADSAFETY are concerning.

Another point is that I didn't make any effort to point this at Drake's versions of dependencies. That might be a non-issue on macOS (using brew in both cases?), but would make it challenging to regenerate an updated version on Linux.

@jwnimmer-tri
Copy link
Collaborator

I've filed #17121 as an attempt to correctly resolve the compilation errors. We can leave this issue to resolve all of the other (possibly runtime-breaking) discrepancies in the petscconf.

@jwnimmer-tri
Copy link
Collaborator

The #17273 and #18038 have landed in the meantime with some improvements.

We are running all Drake regression tests on macOS arm64 as well as running wheel builds, with no sign of any problems. I'll call this fixed well-enough for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters priority: medium type: feature request
Projects
None yet
Development

No branches or pull requests

2 participants