Whoops, good catch. I just copied the flags from what they were in libxc...

I'm rather confused on the flag situation. Points:

• we were getting reports at the forum that the Win conda package (built during the Azure CI) wasn't working for some machines
• a package built after switching the ENABLE_XHOST from /arch:AVX2 to /arch:AVX seemed to work for all
• even before Make ENABLE_XHOST flag checks dependent on compiler brand. #2377, that flag wasn't active: https://dev.azure.com/psi4/psi4/_build/results?buildId=3455&view=logs&j=f31c9f97-4411-58e7-49ac-fc73f645e6b6&t=37c784ef-3eed-5b43-b403-e5baa44c3c05&l=32

2021-11-28T04:42:21.6163025Z -- The CXX compiler identification is Clang 13.0.0 with MSVC-like command-line
2021-11-28T04:42:25.0744510Z -- Check for working C compiler: C:/Program Files/LLVM/bin/clang-cl.exe - skipped
2021-11-28T04:42:25.4493255Z -- Check for working CXX compiler: C:/Program Files/LLVM/bin/clang-cl.exe - skipped
2021-11-28T04:42:26.3103349Z -- Setting option ENABLE_XHOST: ON
2021-11-28T04:42:26.6553541Z -- Performing Test CMAKE_CXX_FLAGS [-march=native] - Success, Appending

• this PR at first commit that adds /arch:AVX to CMAKE_CXX_FLAGS generates a bunch of the below

2021-12-16T22:33:46.0792776Z -- Build files have been written to: D:/a/1/b/build/psi4-core-prefix/src/psi4-core-build
2021-12-16T22:33:46.0793909Z [6/9] Performing build step for 'psi4-core'
2021-12-16T22:33:50.7743244Z [1/1263] Building CXX object src\psi4\libdpd\CMakeFiles\dpd.dir\buf4_mat_irrep_row_zero.cc.obj
2021-12-16T22:33:50.8074886Z clang-cl: warning: argument unused during compilation: '/arch:AVX' [-Wunused-command-line-argument]
2021-12-16T22:33:50.8077002Z [2/1263] Building CXX object src\psi4\libdpd\CMakeFiles\dpd.dir\buf4_mat_irrep_row_wrt.cc.obj
2021-12-16T22:33:50.8080336Z clang-cl: warning: argument unused during compilation: '/arch:AVX' [-Wunused-command-line-argument]
2021-12-16T22:33:50.8416165Z [3/1263] Building CXX object src\psi4\libdpd\CMakeFiles\dpd.dir\buf4_mat_irrep_row_init.cc.obj

So perhaps the original #2292 fix was a dud, and we've just gotten lucky about being built on older AVX nodes where march=native does the trick. (We may have been helped for this by using an old build env that they're about to retire.)

Ok, I think I understand things better now.

• pre Make ENABLE_XHOST flag checks dependent on compiler brand. #2377, the windows build seemed to be getting -march=native. clang-cl does seem to understand march (https://reviews.llvm.org/D51806) so it accepts the flag. But it probably doesn't understand native (https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20150629/132028.html). Also see quote below from https://stackoverflow.com/a/62493711 . So we were probably getting lucky about building on AVX and supposedly fixing the Win conda binary compatibility problem.

And nothing in MSVC has any support for making binaries optimized to run on the computer you build them on. That makes sense, it's designed for a closed-source binary-distribution model of software development.

• nevertheless, from an independent compile someone reported on the forum Win compatibility thread, it does seem that arch AVX, not AVX2, is the solution.
• enter Make ENABLE_XHOST flag checks dependent on compiler brand. #2377 that probably only changed the situation to me reading the code, not at all to the build product since the ENABLE_XHOST was picking up march=native.
• me adding -DCMAKE_CXX_FLAGS=/arch:AVX in the first commit of this PR led to the unused arg warning below (after also turning on printing for Ninja). I now believe the warning was triggered because clang-cl recognizes march (quote below from link above), doesn't act on it, but does drop arch. At one point, I thought the unused flag was because it was presented in the wrong format, but a section in the clang-cl manual gives a list, and arch is fine (https://clang.llvm.org/docs/UsersManual.html#clang-cl).

Yes, indeed. The code in clang/lib/Driver/ToolChains/Arch/X86.cpp makes that –march is always parsed, leaving out /arch unused, no matter in which order they appear.

[1/1263] C:\PROGRA~1\LLVM\bin\clang-cl.exe   -TP -DUSING_LAPACK_MKL -D_USE_MATH_DEFINES -ID:\a\1\s\psi4\include -ID:\a\1\s\psi4\src -imsvc C:\tools\miniconda3\Library\include -imsvc C:\tools\miniconda3\include -imsvc C:\tools\miniconda3\Library\include\eigen3 /arch:AVX -march=native /O2 /Ob2 /DNDEBUG -MD   /EHsc -Xclang -fopenmp -std:c++14 /showIncludes /Fosrc\psi4\libdpd\CMakeFiles\dpd.dir\contract222.cc.obj /Fdsrc\psi4\libdpd\CMakeFiles\dpd.dir\dpd.pdb -c -- D:\a\1\s\psi4\src\psi4\libdpd\contract222.cc
clang-cl: warning: argument unused during compilation: '/arch:AVX' [-Wunused-command-line-argument]

• On to properly forcing Win to build for AVX:
  • (1) recognize the compiler flavor. apparently https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER_FRONTEND_VARIANT.html is the way to discriminate between clang (gnu-imitating) and clang-cl (msvc-imitating), so I've expanded the ENABLE_XHOST logic. I also switched out STREQUAL for MATCHES, @susilehtola, as it seems safer given the diversity (https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER_ID.html).
  • (2) part (1) alone doesn't help because hand-specified CMAKE_CXX_FLAGS=/arch=AVX gets appended by the ENABLE_XHOST-resolved flag, so the hand-specified AVX gets clobbered by AVX2 again. the PR now turns off ENABLE_XHOST and passes AVX. The sensible command line is below.

[1/1263] C:\PROGRA~1\LLVM\bin\clang-cl.exe   -TP -DUSING_LAPACK_MKL -D_USE_MATH_DEFINES -ID:\a\1\s\psi4\include -ID:\a\1\s\psi4\src -imsvc C:\tools\miniconda3\Library\include -imsvc C:\tools\miniconda3\include -imsvc C:\tools\miniconda3\Library\include\eigen3 /arch:AVX /O2 /Ob2 /DNDEBUG -MD   /EHsc -Xclang -fopenmp -std:c++14 /showIncludes /Fosrc\psi4\libdpd\CMakeFiles\dpd.dir\contract444.cc.obj /Fdsrc\psi4\libdpd\CMakeFiles\dpd.dir\dpd.pdb -c -- D:\a\1\s\psi4\src\psi4\libdpd\contract444.cc

I should probably turn off verbose compile in general, then this PR is rtg.

@loriab thanks for a thorough explanation!

I guess these fixes should also get backported in libxc's cmake...

I guess these fixes should also get backported in libxc's cmake...

yes. It's getting long enough I'd almost rather do a separate file under cmake/. The option_with_flags at this point is mostly only doing nice printing. I think I owe you a cmake fix anyways.

Not related to the issue at hand, but since changes are made to it:
On Apple Silicon ARM the -march=native flag is not recognized with Apple's clang 13.
It could e.g. be disabled with an if (NOT CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "arm64") for clang gnu (that's tested) but that would nest if statements even more.

Not related to the issue at hand, but since changes are made to it:
On Apple Silicon ARM the -march=native flag is not recognized with Apple's clang 13.
It could e.g. be disabled with an if (NOT CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "arm64") for clang gnu (that's tested) but that would nest if statements even more.

Thanks, @hokru, do you know if there's a flag for Apple Silicon ARM that should be used in place of march=native? The xhost logic block is running with option_with_flags() (activates the first flag in argument list that the compiler understands), but that's mostly for pretty printing now that there's more compiler logic and only one proposed flag per call. Having started down the logic path, I'm glad to add more cases for Silicon.

I didn't find anything.

There is a related stackexchange discussion https://stackoverflow.com/questions/65966969/why-does-march-native-not-work-on-apple-m1 They suggest the mcpu flag but that's not what we want.

Interesting stuff.

As for the Apple M1, I guess it depends whether we'll be pushing out a released and tested binaries for Apple M1 before clang gets around fixing the defaults, right?