Improve Math.BigMul performance on x64 by Daniel-Svensson · Pull Request #117261 · dotnet/runtime

Daniel-Svensson · 2025-07-03T09:36:39Z

This #115966 but without the mulx support.
See the old PR for benchmarks results and generated code

The reason for opening a separate PR is

Rerun all tests without the mulx code to increase confident that it works on non-AVX2 hardware
Make it easy for the team pick only this specific part of the PR (in case it makes review, benchmarks follow up or similar easier)

Feel free to close it if you prefer to work with the original PR

Copilot

Pull Request Overview

This PR backports the Math.BigMul hardware intrinsic support on x64 without the mulx instruction extension. It adds new BigMul overloads in the X86Base APIs, hooks them into Math.BigMul, and extends the JIT to lower, schedule, and codegen these multi-register intrinsics.

Introduces BigMul methods for 32-, 64-, and pointer-size integers in X86Base and their platform-not-supported stubs.
Updates Math.BigMul to prefer the new X86Base.X64.BigMul path on non-MONO x64, falling back as before.
Enhances JIT (linear scan, lowering, import, list, codegen, tree layout) to recognize and generate BigMul intrinsics.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/System.Private.CoreLib/src/System/Runtime/Intrinsics/X86/X86Base.cs	Added `BigMul` intrinsics for various operand widths
src/System.Private.CoreLib/src/System/Runtime/Intrinsics/X86/X86Base.PlatformNotSupported.cs	Added `BigMul` stubs throwing on unsupported platforms
src/System.Private.CoreLib/src/System/Math.cs	Routed `Math.BigMul` to use the new intrinsics on x64
src/coreclr/jit/lsraxarch.cpp	Updated register allocator for `BigMul` multi-reg defs
src/coreclr/jit/lowerxarch.cpp	Enabled containment checks for `BigMul`
src/coreclr/jit/hwintrinsicxarch.cpp	Imported `BigMul` as a multi-register HW intrinsic
src/coreclr/jit/hwintrinsiclistxarch.h	Listed `BigMul` in the x86 and x64 HW intrinsic tables
src/coreclr/jit/hwintrinsiccodegenxarch.cpp	Emitting `MUL/IMUL` sequence for `BigMul`
src/coreclr/jit/hwintrinsic.h	Updated multi-reg return count for `BigMul`
src/coreclr/jit/gentree.cpp	Defined struct layout for `BigMul` return

Comments suppressed due to low confidence (3)

src/libraries/System.Private.CoreLib/src/System/Math.cs:205

Consider adding unit tests that validate the new Math.BigMul(ulong, ulong, out ulong) path on x64 and ensure correct behavior when X86Base.X64.IsSupported is true/false.

#if !MONO // X64.BigMul is not yet implemented in MONO

JulieLeeMSFT · 2025-07-28T13:48:33Z

@EgorBo, PTAL.

src/coreclr/jit/hwintrinsiccodegenxarch.cpp

JulieLeeMSFT · 2025-11-03T17:18:06Z

Rerunning failed test.

EgorBo · 2026-01-12T03:09:19Z

...ies/System.Private.CoreLib/src/System/Runtime/Intrinsics/X86/X86Base.PlatformNotSupported.cs

+        /// <remarks>
+        ///   <para>Its functionality is exposed in the public <see cref="Math" /> class.</para>
+        /// </remarks>
+        internal static (uint Lower, uint Upper) BigMul(uint left, uint right) { throw new PlatformNotSupportedException(); }


Why do we need these if we don't use them and they're internal?

no, they (int and nint overloads) can probably be removed since we do not have any such public api unless the corresponding API proposal is approved (#58263)

They were there to follow the pattern of other hardware methods and because 32bit bigmul used them before implementing it in JIT instead.

Comment on the other PR:

The nint and 32 bit version never gets called, should I remove them or keep them for the symmetry ?
I do no longer think it makes much sense to make this methods public >(#58263) since you can just call existing BigMul methods, and the nint version was solved by a simple *if * in #114731

I guess it means that NI_X86Base_BigMul should be removed as well ?

I'd remove, it's basically a dead/untested code. Btw, I've pushed some AI-generated tests 🙂

I've removed the 32bit and IntPtr (nint) overloads as well as NI_X86Base_BigMul .

I did not look that much on the tests but thy seems fine (during developmen one error where register was swapped was detected by xxhash beeing used in tests and not by existing BigMul test. It seems like it should be covered by one of the new tests)

…x86_bigmul

EgorBo · 2026-02-23T02:13:16Z

Sorry for the slow feedback loop, I think it's looking good, let's see what CI thinks

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Copilot · 2026-02-23T15:29:04Z

src/tests/JIT/HardwareIntrinsics/X86/Shared/BigMul.cs

+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Xunit;
+
+namespace JIT.HardwareIntrinsics.X86
+{
+    public class BigMulTestSuite
+    {


The test file BigMul.cs is not referenced in any .csproj file, which means it won't be compiled or executed as part of the test suite. You need to either:

Add a new test project (e.g., X86Base.X64_BigMul.csproj) that includes this file, or

Add this file to an existing X86Base.X64 test project

Looking at similar tests in the X86Base.X64 directory, you would typically need to create a .csproj file that includes both this test file and the Program.cs files.

Copilot · 2026-02-23T15:29:05Z

src/tests/JIT/HardwareIntrinsics/X86/Shared/BigMul.cs

+        /// <summary>
+        /// Supplementary test to ensure that the 'out' parameter behavior
+        /// handles stack-allocated variables correctly.
+        /// </summary>
+        private static void TestOutParameterConsistency()
+        {
+            long a = 123456789;
+            long b = 987654321;
+            long lowExpected = a * b;
+
+            Math.BigMul(a, b, out long lowActual);
+
+            Assert.Equal(lowExpected, lowActual);
+        }
+
+        /// <summary>
+        /// Test using properties instead of fields to ensure getter logic 
+        /// doesn't interfere with the intrinsic mapping.
+        /// </summary>
+        private class PropertyContainer
+        {
+            public long Val => 0x1122334455667788;
+        }
+
+        private static void TestPropertyInputs()
+        {
+            var p = new PropertyContainer();
+            long low;
+            long high = Math.BigMul(p.Val, 2, out low);
+
+            Assert.Equal(0L, high);
+            Assert.Equal(p.Val * 2, low);
+        }


The test methods TestOutParameterConsistency and TestPropertyInputs are defined but never called from TestEntryPoint, so they won't be executed during test runs. Either add calls to these methods in TestEntryPoint (lines 30-42), or remove these methods if they're not needed.

Suggested change

/// <summary>

/// Supplementary test to ensure that the 'out' parameter behavior

/// handles stack-allocated variables correctly.

/// </summary>

private static void TestOutParameterConsistency()

{

long a = 123456789;

long b = 987654321;

long lowExpected = a * b;

Math.BigMul(a, b, out long lowActual);

Assert.Equal(lowExpected, lowActual);

}

/// <summary>

/// Test using properties instead of fields to ensure getter logic

/// doesn't interfere with the intrinsic mapping.

/// </summary>

private class PropertyContainer

{

public long Val => 0x1122334455667788;

}

private static void TestPropertyInputs()

{

var p = new PropertyContainer();

long low;

long high = Math.BigMul(p.Val, 2, out low);

Assert.Equal(0L, high);

Assert.Equal(p.Val * 2, low);

}

src/libraries/System.Private.CoreLib/src/System/Math.cs

Copilot · 2026-02-23T15:29:05Z

src/coreclr/jit/hwintrinsiccodegenxarch.cpp

+            var_types   targetType = node->GetSimdBaseType();
+            GenTree*    regOp      = node->Op(1);
+            GenTree*    rmOp       = node->Op(2);
+            instruction ins        = HWIntrinsicInfo::lookupIns(intrinsicId, targetType, m_compiler);
+
+            emitAttr attr = emitTypeSize(targetType);
+            emitter* emit = GetEmitter();


Local variable 'targetType' shadows the function parameter of the same name (line 2484). Consider using a different name like 'operandType' or 'mulTargetType' to avoid confusion. The same applies to 'emit' which shadows the function-scope variable at line 2486.

Suggested change

var_types targetType = node->GetSimdBaseType();

GenTree* regOp = node->Op(1);

GenTree* rmOp = node->Op(2);

instruction ins = HWIntrinsicInfo::lookupIns(intrinsicId, targetType, m_compiler);

emitAttr attr = emitTypeSize(targetType);

emitter* emit = GetEmitter();

GenTree* regOp = node->Op(1);

GenTree* rmOp = node->Op(2);

instruction ins = HWIntrinsicInfo::lookupIns(intrinsicId, baseType, m_compiler);

emitAttr attr = emitTypeSize(baseType);

src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/X86/X86Base.cs

Add X86Base.BigMul and call it from Math.BigMul

d781b3a

Copilot AI review requested due to automatic review settings July 3, 2025 09:36

github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 3, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 3, 2025

Copilot AI reviewed Jul 3, 2025

View reviewed changes

This was referenced Jul 3, 2025

browser-wasm Windows build error #116746

Closed

wasm build failure in CI #117017

Open

Daniel-Svensson mentioned this pull request Jul 3, 2025

Improve Math.BigMul on x64 by adding new internal Multiply hardware intrinsic to X86Base #115966

Closed

JulieLeeMSFT requested a review from EgorBo July 28, 2025 13:48

EgorBo reviewed Jul 31, 2025

View reviewed changes

src/coreclr/jit/hwintrinsiccodegenxarch.cpp Show resolved Hide resolved

Daniel-Svensson added 2 commits August 4, 2025 20:34

merge upstream/main

90d2934

add comment regarding swap

e110d32

build-analysis bot mentioned this pull request Aug 4, 2025

System.Data.OleDb.Tests timeout in net48 x86 Release leg #87783

Closed

Merge branch 'main' into x86_bigmul

69494cb

build-analysis bot mentioned this pull request Oct 11, 2025

Assert Interop.Sys.LSeek(this, 0, Interop.Sys.SeekWhence.SEEK_CUR) >= 0 in System.Net.XXX tests #120577

Closed

JulieLeeMSFT added this to the 11.0.0 milestone Oct 13, 2025

EgorBo added 3 commits January 12, 2026 03:50

Merge branch 'main' of https://github.com/dotnet/runtime into x86_bigmul

dae319b

Update hwintrinsicxarch.cpp

a1ddb63

Merge branch 'main' into x86_bigmul

b41469b

EgorBo reviewed Jan 12, 2026

View reviewed changes

EgorBo and others added 4 commits January 12, 2026 04:24

Merge branch 'x86_bigmul' of github.com:Daniel-Svensson/runtime into …

c50082a

…x86_bigmul

Add a test

c18eb40

Merge remote-tracking branch 'upstream/main' into x86_bigmul

571d2c4

Remove 32bit overload variant of BigMul

ef4bf17

Daniel-Svensson requested a review from EgorBo January 18, 2026 08:50

Merge branch 'main' into x86_bigmul

ff84cf2

build-analysis bot mentioned this pull request Feb 23, 2026

[android][arm64] System.Net.Sockets.Tests.SendTo_SyncForceNonBlocking.Datagram_UDP_ShouldImplicitlyBindLocalEndpoint fails with NetworkUnreachable #120526

Open

Update hwintrinsiccodegenxarch.cpp

044f2eb

EgorBo requested a review from Copilot February 23, 2026 15:23

Copilot started reviewing on behalf of EgorBo February 23, 2026 15:24 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Conversation

Daniel-Svensson commented Jul 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

JulieLeeMSFT commented Jul 28, 2025

Uh oh!

Uh oh!

JulieLeeMSFT commented Nov 3, 2025

Uh oh!

EgorBo Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Daniel-Svensson Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Daniel-Svensson Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Daniel-Svensson Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

EgorBo commented Feb 23, 2026 •

edited

Loading