Skip to content

build: support building on POWER/ppc64 and other non-x86 targets - #331

Open
Scottcjn wants to merge 1 commit into
rurban:masterfrom
Scottcjn:fix/build-on-power-ppc64
Open

build: support building on POWER/ppc64 and other non-x86 targets#331
Scottcjn wants to merge 1 commit into
rurban:masterfrom
Scottcjn:fix/build-on-power-ppc64

Conversation

@Scottcjn

Copy link
Copy Markdown
Contributor

SMHasher does not currently build on POWER (ppc64 / ppc64le) or other non-x86 targets. Two issues:

  1. The GNU/Clang path unconditionally adds -march=native. POWER gcc rejects that flag (it uses -mcpu=native), so the flag is injected and every compile fails. This now probes both with check_cxx_compiler_flag and uses whichever the compiler accepts. x86 and aarch64 are unaffected (they accept -march=native as before).

  2. PMP_Multilinear uses x86 SSE intrinsics and x86 inline asm. It was excluded for aarch64 but not for other non-x86 targets, so on ppc64 it fell through to the x86 path and failed to compile (unknown register name 'edx' in 'asm'). It is now restricted to x86 (and the existing 32-bit arm path). So the CMake source decision and the main.cpp registration can never disagree, CMake defines -DHAVE_PMPML when the sources are compiled and main.cpp gates the table entry on that single macro.

Testing

Built and tested on real POWER8 hardware, both endiannesses:

  • ppc64 big-endian (POWER8, gcc 13.3): configures, builds, and runs.
  • ppc64le (POWER8, gcc 10): configures, builds, and runs; sumhash, BadHash, sumhash32 and the rest verify the same as on x86.

x86_64 is unchanged: it still uses -march=native and still builds and registers PMP_Multilinear.

Side note from the big-endian run: a number of hashes that read or write multi-byte values in native byte order fail verification on big-endian (their verification value comes out byte-reversed). That is a separate correctness issue and I will open it with a fix; this PR only makes the project build on these targets.

The GNU/Clang path always added -march=native, which POWER gcc
rejects (it uses -mcpu=native). Probe both with
check_cxx_compiler_flag and use whichever the compiler accepts;
x86 and aarch64 are unaffected.

PMP_Multilinear uses x86 SSE intrinsics and x86 inline asm, so it
only builds on x86 (and the existing arm path). It was excluded for
aarch64 but not for other non-x86 targets, so ppc64 fell through to
the x86 asm and failed to compile. Restrict it to x86/arm in
CMakeLists and main.cpp so it is skipped elsewhere.

Builds and runs on native big-endian ppc64 (POWER8).
@rurban

rurban commented Jun 27, 2026

Copy link
Copy Markdown
Owner

I like that. But first the windows compiler needs to be bumped.

@Scottcjn

Copy link
Copy Markdown
Contributor Author

Sounds good, thanks. I will wait on the Windows compiler bump. If it helps, I can re-run the big-endian verification on real PowerPC hardware once it lands.

@rurban

rurban commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Na, too busy. Will fix windows later

@Scottcjn

Copy link
Copy Markdown
Contributor Author

On the Windows compiler bump, I dug into it so you do not have to. It is not caused by this PR.

The runner image has been re-provisioned. The job log's Runner Image block says:

Image: windows-2025-vs2026

So windows-2025 no longer carries VS 2022, and .github/workflows/ci.yml pins the generator:

- run: cmake -G "Visual Studio 17 2022" -A x64 -DCMAKE_BUILD_TYPE=Release .

CMake stops at project() on CMakeLists.txt line 11 with "Generator Visual Studio 17 2022 could not find any instance of Visual Studio". It never gets as far as the compiler flag probe or the PMPML gating this PR changes.

It is already red on master for the same reason: run 28703089508 on a27c3a5 has windows failing and the other seven jobs green. This PR shows the identical pattern, windows red and everything else green including aarch64 and armv7.

The fix I would suggest is dropping the pin rather than moving it to 18:

- run: cmake -A x64 -DCMAKE_BUILD_TYPE=Release .

CMake then selects the newest Visual Studio actually installed, so it survives the next image change too instead of trading a hardcoded 17 for a hardcoded 18.

Do you want me to push that into this PR, or keep CI changes out of it and let you take it separately? Either is fine, just say which.

Nothing else needs doing on the PR side: neither CMakeLists.txt nor main.cpp has moved on master since the branch point, so there is no rebase to do.

One correction to what I offered in June. I said I could re-run the big-endian verification on real PowerPC once the Windows side landed. The POWER8 box is powered down at the moment, so I cannot re-run it today. The results in the description stand as recorded from the original run, and I will re-confirm when the machine is back up if you want that before merging.

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.

2 participants