Skip to content

Update Benchstones (BenchI & BenchF) to validate results #125383

Closed
abdulrahmanhossam wants to merge 4 commits intodotnet:mainfrom
abdulrahmanhossam:feature/issue-5049-bench-validation
Closed

Update Benchstones (BenchI & BenchF) to validate results #125383
abdulrahmanhossam wants to merge 4 commits intodotnet:mainfrom
abdulrahmanhossam:feature/issue-5049-bench-validation

Conversation

@abdulrahmanhossam
Copy link

@abdulrahmanhossam abdulrahmanhossam commented Mar 10, 2026

Fixes #5049

This PR completes the correctness validation for the BenchI (Integer) and BenchF (Floating Point) suites. Previously, many of these benchmarks only measured performance while silently returning true. This update ensures that the JIT compiler produces mathematically correct results without affecting the benchmark's performance profile by using O(1) post-loop checks.


Key Changes:

1. BenchI (Integer) Suite

  • AddArray / AddArray2: Validated final accumulated indices and values, including intentional 32-bit overflows (e.g., verifying that a[200][200] resolves correctly after overflow).
  • MulMatrix: Validated the 900jk mathematical derivation for the result matrix.
  • NDhrystone: Verified global state consistency (Iterations + 10).
  • IniArray: Added post-loop check to verify successful array initialization.

2. BenchF (Floating Point) Suite

  • Root Finding (Bisect, NewtR, Regula, Secant, NewtE): Added validation for the final roots of the equations. For example, verifying the Plastic Ratio ($\approx 1.32471$) for the equation $x^3 - x - 1 = 0$.
  • Integration (Romber, Simpsn, Trap): Verified the definite integral results for $e^{-x^2}$ and $e^{-2x}$ against their analytical values.
  • Linear Algebra (InvMt, MatInv4, SqMtx): Added checks for matrix inversion accuracy by verifying the resulting Identity Matrix and Bit Error Rate (BER).
  • Chaos & Signal Processing (Lorenz, FFT, DMath): Validated final attractor coordinates for Lorenz and frequency domain components (DC offset) for FFT.
  • Whetstone: Added a precision check for the circular Exp(Log(x)) transformations in Module 11 to ensure identity stability.

Technical Implementation Details:

  • Zero Overhead: All validations are performed strictly outside the timed benchmark loops to ensure no impact on JIT code quality measurements.
  • Floating Point Tolerance: Applied appropriate epsilon values (ranging from 1e-5 to 1e-10) to account for precision differences across different hardware architectures (x64, ARM64).
  • Error Handling: Updated goto error labels (e.g., in MatInv4) to return false upon numerical failure instead of a silent true, ensuring test reliability.
  • Audited & Verified: Audited the entire directory; files already containing internal verification (like Adams, InProd, and LLoops) were kept as-is to maintain original logic.

Checklist:

  • All O(1) checks are outside the performance-critical loops.
  • Verified that the benchmarks still pass under normal conditions.
  • No new dependencies added.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 10, 2026
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 10, 2026
@EgorBo
Copy link
Member

EgorBo commented Mar 10, 2026

To be honest, I don't think these tests add much value (besides checking JIT asserts). It's fine to add a validation, but this PR does a lot of absolutely unnecessary indention changes + adds magic numbers like
{8E09F562-AF35-43EA-935B-906631F12E86}

We prefer first-time contributors to focus on rather simpler/cleaner changes.

@abdulrahmanhossam
Copy link
Author

Hi @EgorBo,

Thanks for the feedback. Regarding the indentation changes, these were automatically applied by the IDE's formatter on save to adhere to standard C# styles. I understand this makes the diff noisier than intended, so I will revert the unnecessary styling changes and keep the focus strictly on the validation logic.

As for the "magic numbers," these are the mathematically expected reference values for these specific algorithms (e.g., the Lorenz attractor's steady state or the Plastic Ratio for root-finding). While these tests do catch JIT asserts, adding correctness validation ensures that the JIT isn't just "running" the code, but generating correct numerical output—which is the core purpose of #5049.

I will clean up the PR by:

  1. Reverting all unnecessary whitespace/indentation changes.
  2. Replacing "magic numbers" with descriptive constants or adding comments to explain the origin of these reference values.

I believe even for a first-time contributor, ensuring the correctness of these long-standing benchmarks is a valuable goal. Updated commits coming soon.

@abdulrahmanhossam
Copy link
Author

Closing this for now, might revisit later

@abdulrahmanhossam abdulrahmanhossam deleted the feature/issue-5049-bench-validation branch March 10, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update BenchI and BenchF benchmarks to validate results

2 participants