Skip to content

Use __int128 for generic umul_ppmm and smul_ppmm - #2820

Open
darkavatar23 wants to merge 1 commit into
flintlib:mainfrom
darkavatar23:int128-umul-ppmm
Open

Use __int128 for generic umul_ppmm and smul_ppmm#2820
darkavatar23 wants to merge 1 commit into
flintlib:mainfrom
darkavatar23:int128-umul-ppmm

Conversation

@darkavatar23

Copy link
Copy Markdown

The generic fallback for umul_ppmm/smul_ppmm in src/longlong.h builds a 64x64 -> 128 product out of four half-word multiplications. Where the compiler has a 128-bit integer type, one widening multiply does the same job. Platforms that already have inline asm for umul_ppmm are untouched: the new block sits behind !defined(umul_ppmm), like the generic one it precedes.

This is not a rare path, at least under gcc. Even with assembly enabled, gcc has inline asm for umul_ppmm only on amd64/i386 (longlong_asm_gcc.h:21) and arm/aarch64 (:247), so riscv64, ppc64, s390x, loongarch64 and mips64 always get the generic block; and ./configure --disable-assembly puts every architecture on it, x86-64 included. None of this applies to clang, which gets longlong_asm_clang.h unconditionally (src/longlong.h:47-48), outside the FLINT_WANT_ASSEMBLY gate.

That last one makes the change easy to see without RISC-V hardware. On x86-64 with gcc, configure with --disable-assembly and preprocess a file that uses the macro, say src/ulong_extras/mulmod_preinv.c: before the patch umul_ppmm expands to the four-multiplication version (grep -c __x0 gives 8, __rx 0), after it to a single __uint128_t product (__rx 8, __x0 0). Both configurations build and pass make check for ulong_extras, nmod, nmod_mat, nmod_vec, fmpz and mpn_extras: 355 tests, same count either way.

FLINT already uses the same widening multiply on the generic GNU path elsewhere: n_mulhi (src/ulong_extras.h:47-48), ull_t (src/nmod.h:296-299) and _mul (src/crt_helpers.h:309-315). The guard here is copied from crt_helpers.h:309.

For gcc this also completes the umul_ppmm and smul_ppmm items ticked off in #1636, which until now only took effect for clang through longlong_asm_clang.h:194-195.

Codegen

gcc 16, -O2, counting the final ret:

target generic with this patch
x86-64 28 instructions, 4 imul 5 instructions, 1 mul
riscv64 -march=rv64gc 26 instructions, 4 mul 5 instructions, mul + mulhu

smul_ppmm on riscv64 goes from 32 instructions and 4 mul to mul + mulh. Both mulhu and mulh belong to the base M extension, so nothing beyond a stock rv64gc build is needed.

Benchmark

SiFive U74 (VisionFive 2, rv64gc), gcc 16 cross build, governor pinned to performance at 1.5 GHz, master and patched runs interleaved, 4 runs each, spread <= 0.4%. Modulus a 60-bit prime:

master patched speedup
nmod_mat_mul, 200x200 0.4894 s 0.2133 s 2.29x
nmod_poly_gcd, length 800 0.04798 s 0.04690 s 1.02x
nmod_poly_mul, length 4000 0.2172 s 0.2152 s 1.01x

The gain is not uniform. It is large where nmod_mul dominates the inner loop and almost nothing for the polynomial routines, which spend their time elsewhere.

Both libraries were built from 00e660a with ./configure --host=riscv64-linux-gnu --disable-shared --disable-pthread, with only src/longlong.h differing between them.

Division is deliberately left alone: the same treatment applied to udiv_qrnnd makes gcc emit calls to __udivti3 and __umodti3 instead of the hardware divide, so it would be a pessimisation.

Testing

Test suites cross-compiled for riscv64 and run natively on the U74 against the patched library: test (which contains t-umul_ppmm and t-smul_ppmm), nmod_mat, nmod, nmod_vec, ulong_extras, fmpz, mpn_extras. All exit 0, 371 PASS in total, no failures.

Worth flagging: no CI job compiles the new branch. On every platform CI covers, umul_ppmm is already defined before this point, by inline asm on x86-64/aarch64 with autotools, by longlong_asm_clang.h for clang, by longlong_msc_*.h for MSVC, while the 32-bit Alpine job has no __int128 at all. The ways to exercise it are --disable-assembly or an architecture without inline asm.

One regression I found

On sparc64, gcc compiles the 128-bit product into a call __multi3 rather than inlining it (gcc 15.2 at -O2, also with -mcpu=niagara4). Every other target I checked inlines it: riscv64, ppc64, s390x, loongarch64, mips64el, aarch64. Since FLINT has no sparc CI and no sparc-specific code I left the guard as it is, but I am happy to add && !defined(__sparc__) if you prefer.

@albinahlback

Copy link
Copy Markdown
Collaborator

Yeah, I think this is good. Do you have any benchmark on x86 comparing inline assembly versus this short 128-bit arithmetic? As Fredrik mentioned here, it was slower on older GCC, but GCC has improved on this area then I'd like to remove the inline assembly.

@darkavatar23

Copy link
Copy Markdown
Author

I did not have x86 numbers, so I measured it (with my local infrastucture) -- on every x86 box I could get my hands on: Zen 2 (Ryzen 3800XT), Raptor Lake (i7-13700KF), Haswell (i5-4590S) and Sandy Bridge-EP (Xeon E5-2650, over proxmox), across gcc 12.2 / 14.2 / 15.2 / 16.2.

Method: default ./configure, plus an #undef umul_ppmm / #undef smul_ppmm right before the block this PR adds, so exactly these two macros resolve to the __int128 versions while everything else in longlong.h (add_ssaaaa and friends) stays inline asm. Every forced-__int128 build passes make check MOD=test, which includes t-umul_ppmm and t-smul_ppmm. Timings are best-of-5 loops per process, 4 interleaved process runs per side, pinned to one core.

Codegen first: for these two macros gcc already emits exactly what the asm does -- a single mulq for umul_ppmm, a single imulq for smul_ppmm (checked on gcc 12/14/16), and with -mbmi2 it switches to mulx on its own, matching the BMI2 asm variant.

End to end, nmod_mat_mul 200x200 (modulus near 2^60), range over the 4 runs (fastest - slowest):

machine gcc asm __int128
Zen 2 16.2.1 0.0173-0.0175 s 0.0165-0.0169 s (~5% faster)
Zen 2 12.2.0 0.0168-0.0169 0.0163-0.0164 (~3% faster)
Raptor Lake 14.2.0 0.0141-0.0145 0.0136-0.0140 (~3% faster)
Haswell 15.2.0 0.0278-0.0279 0.0273-0.0274 (~2% faster)
Sandy Bridge-EP 14.2.0 0.0427-0.0428 0.0429-0.0430 (~0.6% slower)

nmod_poly_gcd is a wash everywhere. nmod_poly_mul is a wash on the modern cores but favors the asm by 1-3% on Haswell and Sandy Bridge (0.0073 vs 0.0075 s on the i5).

So: on 2012-2014 cores the two forms trade blows within ~3% either way; on anything recent the __int128 version is never slower and nmod_mat_mul is consistently 3-5% faster -- plausibly because the multiply is visible to the optimizer instead of being an opaque asm pinned to rax/rdx. If you do end up dropping the x86 inline asm for these two macros, the block this PR adds is exactly what gcc builds would fall back to, and per the above that fallback costs nothing on current hardware.

Caveats: the Raptor Lake numbers are from a Linux environment under WSL2 and the Xeon is a live virtualization host (its first warm-up run was discarded) -- but every comparison is A/B inside the same environment, so the deltas hold. And this only covers umul_ppmm/smul_ppmm: I did not touch add_ssaaaa and the other carry-chain macros, which is where I would guess the regression Fredrik measured in #1636 came from.

Sorry for the 6 hours waiting it took me a while to run and configure the environments.

@albinahlback

Copy link
Copy Markdown
Collaborator

Sounds great! Can you remove the inline assembly definitions of umul_ppmm and smul_ppmm, so they use the int128 version?

@fredrik-johansson

Copy link
Copy Markdown
Collaborator

For the generic fallback code this is certainly fine, but there are indeed several places where I've measured a slowdown on x86-64. A recently commented example is ulong_extras/ll_is_prime.c:

/* GCC 11/Zen 3: the following functions are faster implemented with
   umul_ppmm/add_ssaaaa inline assembly than with __uint128_t. */

We'd have to recheck whether this applies when the umul_ppmm alone uses __uint128 while keeping add_ssaaaa inline assembly. Other modules that may need some microbenchmarks are nfloat and mpn_mod. Though if the difference is <5%, it may not be worth the hassle to maintain inline assembly versions; one can maybe accept a small slowdown and bet on compilers improving their codegen...

@albinahlback

Copy link
Copy Markdown
Collaborator

I would guess that umul_ppmm using int128 is pretty safe since the expansion is very simple. However, the add_s...a... expansion is trickier if the compiler cannot handle CPU flags well or knows how to convert to a carry chain properly -- this one I'm not so sure about for GCC and I would say we keep until experiments show something else.

@darkavatar23

Copy link
Copy Markdown
Author

Sure. How about this:

  • amd64: remove both macros' asm, including the BMI2 mulx variant (per the codegen check above, gcc already emits mulx on its own with -mbmi2).
  • aarch64: the __int128 block applies there too, but I have not measured it yet -- I'll run the same A/B on the arm64 hardware I have and drop the asm only if the numbers hold.
  • 32-bit x86 and arm keep their asm: FLINT_BITS == 32 there, so the __int128 block never kicks in and removing the asm would drop them to the generic 4-multiplication fallback.
  • longlong_msc_x86.h is _umul128 intrinsics, not inline asm -- untouched.
  • add_ssaaaa and the other carry chains stay exactly as they are.

Per Fredrik's point, before removing anything I'll microbenchmark ll_is_prime, nfloat and mpn_mod with exactly this split (only umul_ppmm/smul_ppmm on __int128, add_ssaaaa still asm) -- Zen 2 is the closest I have to the Zen 3 note in ulong_extras/ll_is_prime.c -- and post the numbers here.

i have a zen4, but it's far from zen3.
image

Do you want the removal in this same PR or in a follow-up? Fredrik said this one is fine as-is, so either works for me.

@fredrik-johansson

Copy link
Copy Markdown
Collaborator

OK, my up to date view is that we should not drop the inline assembly umul_ppmm for x86-64. I've had Claude check both variants for a bunch of dot product-type kernels routines and the __uint128 version often performs 1.5x worse. Here is Claude's precise analysis of why: disassembly shows identical multiply and adc counts but increased register-register moves: "GCC's u128 lowering doesn't pin the product halves where the subsequent add_ssaaaa/add_sssaaaaaa asm blocks want them, and the shuffling lands on the critical carry path of the inner loop".

@albinahlback

Copy link
Copy Markdown
Collaborator

Great job looking into this. I did not expect GCC to behave like this (really, removing register-register moves is a simple compiler routine).

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.

3 participants