Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced ASSERT function with a macro and enabled LTO #24

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tdulcet
Copy link
Member

@tdulcet tdulcet commented Apr 14, 2024

  • Updated the ASSERT function to use a macro instead, similar to the C standard library assert macro in assert.h.
    • This should reduce the number of false positive warnings, as compilers and static analyzers can much more easily check if the assert is going to fail.
    • This should also result in a performance improvement, as it no longer needs to call a function to test the expression.
    • It seems to enable more loop unrolling, as evident by the required changes to the inline assembly. Thanks to @ldesnogu for help with that in Fixed assorted compiler warnings #22.
  • Updated the makemake.sh script to enable Link Time Optimization (LTO) by default.

These ASSERT function changes were originally part of #22, but I had to remove them due it causing #10 on x86 systems when compiled with GCC:

INFO: Maximum recommended exponent for FFT length (3 Kdbl) = 66742; p[ = 66431]/pmax_rec = 0.9953402655.
Initial DWT-multipliers chain length = [hiacc] in carry step.
M66431: using FFT length 3K = 3072 8-byte floats, initial residue shift count = 16763
This gives an average   21.624674479166668 bits per digit
Using complex FFT radices        12         8        16
mers_mod_square: radix0/2 not exactly divisible by NTHREADS - This will hurt performance.
Using 4 threads in carry step
ERROR: Function mi64_shlc, at line 457 of file ../src/mi64.c
Assertion 'cy == 0ull' failed: Nonzero carryout of nonoverlapping vector add!

This should NOT be merged until after #10 is fixed.

Base automatically changed from warnings to main August 3, 2024 16:24
@ldesnogu ldesnogu changed the base branch from main to singlethread August 4, 2024 22:03
@ldesnogu ldesnogu changed the base branch from singlethread to main August 4, 2024 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant