Itk: Apple Silicon support

[thewtex]

Build and test on the new Apple M1 ARM Silicon processor / system.

Description

We will have to work with the newer Xcode / processor / binary system.

Additional Information

This is a tracking issue to discuss issues / progress.

CC: @dzenanz @imikejackson

[imikejackson]

Just submitted PR #2251 which detects the arm64 target and allows the compilation to proceed farther.

[imikejackson]

Trying to compile ITK on Apple Si (13" MacBook Pro M1) gives the following compile error:

```FAILED: Modules/ThirdParty/PNG/src/itkpng/CMakeFiles/itkpng.dir/pngget.c.o
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -Ditkpng_EXPORTS -IModules/ThirdParty/ZLIB/src -I/Users/mjackson/DREAM3D-Dev/ITK-5.2rc01/Source/ITK/Modules/ThirdParty/ZLIB/src -IModules/ThirdParty/PNG/src -I/Users/mjackson/DREAM3D-Dev/ITK-5.2rc01/Source/ITK/Modules/ThirdParty/PNG/src -I/Users/mjackson/DREAM3D-Dev/ITK-5.2rc01/Source/ITK/Modules/ThirdParty/PNG/src/itkpng -mtune=generic -Wno-uninitialized -Wno-unused-parameter -Wall -Wcast-align -Wdisabled-optimization -Wextra -Wformat=2 -Winvalid-pch -Wno-format-nonliteral -Wpointer-arith -Wshadow -Wunused -Wwrite-strings -funit-at-a-time -Wno-strict-overflow -w -O3 -DNDEBUG -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk -mmacosx-version-min=10.15 -fPIC -MD -MT Modules/ThirdParty/PNG/src/itkpng/CMakeFiles/itkpng.dir/pngget.c.o -MF Modules/ThirdParty/PNG/src/itkpng/CMakeFiles/itkpng.dir/pngget.c.o.d -o Modules/ThirdParty/PNG/src/itkpng/CMakeFiles/itkpng.dir/pngget.c.o -c /Users/mjackson/DREAM3D-Dev/ITK-5.2rc01/Source/ITK/Modules/ThirdParty/PNG/src/itkpng/pngget.c
In file included from /Users/mjackson/DREAM3D-Dev/ITK-5.2rc01/Source/ITK/Modules/ThirdParty/PNG/src/itkpng/pngget.c:15:
/Users/mjackson/DREAM3D-Dev/ITK-5.2rc01/Source/ITK/Modules/ThirdParty/PNG/src/itkpng/pngpriv.h:534:16: fatal error: 'fp.h' file not found

include

           ^~~~~~

1 error generated.
```

When I download the source for libPng 1.6.38 and try the CMakeLists.txt to configure and compile I get errors. When just using the ./configure; make; make install I do get a successful compile. So something is up with the libPng in the ARM_NEON defines or the floating point functions...

[dzenanz]

Maybe an #include is missing somewhere?

[dzenanz]

More likely, this issue has something to do with clang version, and not much if anything with ARM architecture.

[dzenanz]

Can you try compiling with Xcode 12.3 on x64 machine?

[thewtex]

Compile errors are addressed with https://github.com/InsightSoftwareConsortium/ITK/pull/2257

[imikejackson]

Just did another compile and the M1 machine was able to compile ITK. Looks like just a few test failures.

[dzenanz]

The following tests FAILED (most likely due to numerical differences):
668 - itkPatchBasedDenoisingImageFilterTest0 (Failed)
669 - itkPatchBasedDenoisingImageFilterTestGaussian (Failed)
670 - itkPatchBasedDenoisingImageFilterTestRician (Failed)
671 - itkPatchBasedDenoisingImageFilterTestPoisson (Failed)
on this machine (uname -a): Darwin appleDTK2.kitware.com 20.3.0 Darwin Kernel Version 20.3.0: Mon Dec 7 22:03:48 PST 2020; root:xnu-7195.80.16.111.1~1/RELEASE_ARM64_T8020 arm64. We might need to follow up later.

[seanm]

We've finally received our Mac Mini with ARM CPU and I'm setting it up as a build bot that will build ITK nightly.

Running into a few issues of course:

1. HDF5 doesn't like multiple arches, upstream WIP patch here: https://github.com/HDFGroup/hdf5/pull/318
2. building as x86_64 on ARM gives clang: error: the clang compiler does not support '-march=corei7' from this: https://github.com/InsightSoftwareConsortium/ITK/blob/master/CMake/ITKSetStandardCompilerFlags.cmake#L246 Not sure why a library like ITK would specify any -march flags at all, seems to me like something the person building the library should decide. But anyway, on ARM, if you do just clang -march=corei7 test.cxx you get that error, but if you do clang -arch x86_64 -march=corei7 test.cxx then that's fine. I guess whatever is passing that -march=corei7 is failing to also pass the CMAKE_OSX_ARCHITECTURES choice?
3. detection of emmintrin.h I think also has problems with CMAKE_OSX_ARCHITECTURES. If I do cmake -D CMAKE_OSX_ARCHITECTURES="x86_64;arm64" ../ITK the output is:

-- Looking for C++ include emmintrin.h
-- Looking for C++ include emmintrin.h - not found

but there can't be a single answer to this check. It's not something that can be checked once at configuration time. The answer is different per-arch, so it needs to be a compile-time check.

I'm going to take a crack at these...

[seanm]

Also:

1. KWSys SystemInformationImplementation::ParseSysCtl here determines a nonsense clock speed for the CPU. I have a patch coming for that. CC @bradking

[thewtex]

Re: emmintrin / VXL, I wonder if this library would work: https://github.com/simd-everywhere/simde

CC @hjmjohnson

[seanm]

For my above list:

1: now merged upstream in https://github.com/HDFGroup/hdf5/pull/318 @thewtex @bradking to get newer HDF into ITK, are these instructions stale? They strike me as old, referring to HDF5 1.8 and svn... Do I in fact just need to edit and run UpdateFromUpstream.sh?

4: merged in ITK release & master in https://github.com/InsightSoftwareConsortium/ITK/pull/2292

[seanm]

Regarding my point 2: @hjmjohnson in 41203a53241f3369fe527d07c2a5e256731f1945 you added the -march=corei7 flag, maybe I'm missing it, but I don't see any conditionalization for Intel there. Wouldn't this be breaking things for all other CPU types, and for cross compilation generally? Seems to me it should just be removed...

[hjmjohnson]

@seanm line 179 below that does an explicit test to determine if the flag is valid. For non-intel cpu's the test should fail and the flags not added.

[dzenanz]

are these instructions stale

Totally. Perhaps that file should be removed. UpdateFromUpstream.sh is the way to go. Sometimes you don't even need to edit it, just run.

[hjmjohnson]

NOTE: HDF5 seems to have changed a lot in the recent updates. I don't think a niave "UpdateFromUpstream.sh" worked when I tried it in mid January.

[seanm]

@hjmjohnson I see. I'm not familiar with exactly what check_cxx_compiler_flags does, but I'm guessing the issue is the fact that the -march=corei7 flag is sometimes valid and sometimes invalid depending on what other flags are also passed. Specifically, on an ARM Mac:

builder@builder19 Desktop % clang -march=corei7 test.cxx 
clang: error: the clang compiler does not support '-march=corei7'

builder@builder19 Desktop % clang -arch x86_64 -march=corei7 test.cxx               

builder@builder19 Desktop % clang -arch arm64 -march=corei7 test.cxx 
clang: error: the clang compiler does not support '-march=corei7'

builder@builder19 Desktop % clang -arch x86_64 -arch arm64 -march=corei7 test.cxx
clang: error: the clang compiler does not support '-march=corei7'

So I guess when I specify CMAKE_OSX_ARCHITECTURES=x86_64 (i.e. building for Rosetta only), the corresponding -arch x86_64 flag is not going to check_cxx_compiler_flags, and, as you see above, with no arch flag it builds only natively, where march=corei7 is invalid.

[seanm]

@dzenanz thanks for confirming, will try that.

[seanm]

@hjmjohnson I'm staying with HDF5 1.10.x, not 10.12.x. I managed to update VTK (with help) it wasn't toooo bad.

[hjmjohnson]

@seanm Your the best! Thank you for digging in the weeds to get this working.

[seanm]

@dzenanz @hjmjohnson so I have HDF5 1.10.6 -> 1.10.7 basically working, but seems they added some new files. If I add them locally, ITK builds. This being 3rd party code, do I just git add these files as usual, or...?

[dzenanz]

git add these files as usual

If they match the pattern which is being copied, yes. If not, you also need to update the pattern.

[seanm]

OK, finally have our first Universal Binary build!

https://open.cdash.org/build/7078642

Of my numbered issues above: 1, 3, and 4 are fixed.

The following tests FAILED:
    607 - itkPatchBasedDenoisingImageFilterTest0 (Failed)
    608 - itkPatchBasedDenoisingImageFilterTestGaussian (Failed)
    609 - itkPatchBasedDenoisingImageFilterTestRician (Failed)
    610 - itkPatchBasedDenoisingImageFilterTestPoisson (Failed)
    1363 - itkBSplineScatteredDataPointSetToImageFilterTest05 (Failed)
    1444 - itkSymmetricEigenAnalysisImageFilterTestOrderByValue (Failed)
    1445 - itkSymmetricEigenAnalysisImageFilterTestOrderByMagnitude (Failed)
    1446 - itkSymmetricEigenAnalysisImageFilterTestDoNotOrder (Failed)

The first 4 test failures were noted weeks ago, not sure what's up with the others.

[seanm]

@hjmjohnson regarding the march=corei7 issue... my preference is to just remove it. For me, it's extremely weird for a library like ITK to specify such a thing. Only the person building the library knows how old a CPU and/or OS he's targeting. The change from https://github.com/InsightSoftwareConsortium/ITK/issues/1114 I find equally wrong. If you compile ITK on a system that happens to support AVX2, why should you then assume that all the CPUs that will run the built result also support AVX2? There seems to be an assumption here that the resulting library/app will only run on the build machine itself. Thoughts?

[thewtex]

I ordered one of the M1 Mac Mini's for local development :green_apple: :apple: :chipmunk: .

[seanm]

I've been so focused on getting things building Universal on arm, I forgot to try building Universal on Intel. Found another issue. https://github.com/InsightSoftwareConsortium/ITK/pull/2450

[thewtex]

With #2467 #2466 we will have a green dashboard build, h/t @imikejackson

[seanm]

The RogueResearch19 builds are indeed mostly green today. Some warnings that are not arm-specific. And some test failures: https://open.cdash.org/viewTest.php?onlyfailed&buildid=7133437 Some (all?) of those should be fixed with #2467 I guess.