Skip to content

sys/div: calculate magic numbers at compile time#9280

Closed
ZetaR60 wants to merge 12 commits intoRIOT-OS:masterfrom
ZetaR60:RIOT_div_magic
Closed

sys/div: calculate magic numbers at compile time#9280
ZetaR60 wants to merge 12 commits intoRIOT-OS:masterfrom
ZetaR60:RIOT_div_magic

Conversation

@ZetaR60
Copy link
Contributor

@ZetaR60 ZetaR60 commented Jun 3, 2018

This is a rewrite of sys/div to provide much simpler division by calculating the required magic numbers at compile time, rather than hard-coding them.

tests/unittests/tests-div is partially working, with a few rounding errors that I need to fix:

[liveuser@localhost-live unittests]$ make BOARD=mega-xplained term
/home/liveuser/Desktop/RIOT_div_magic/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "9600"
No handlers could be found for logger "root"
2018-06-03 17:59:59,585 - INFO # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
2018-06-03 18:00:01,363 - INFO # 

2018-06-03 18:00:01,453 - INFO # main(): This is RIOT! (Version: 2018.07-devel-462-g5e5275-localhost-live-RIOT_div_magic)
2018-06-03 18:00:01,454 - INFO # .Dividing            0 by 15625...
2018-06-03 18:00:01,488 - INFO # Dividing            1 by 15625...
2018-06-03 18:00:01,554 - INFO # Dividing           10 by 15625...
2018-06-03 18:00:01,554 - INFO # Dividing           32 by 15625...
2018-06-03 18:00:01,588 - INFO # Dividing        15625 by 15625...
2018-06-03 18:00:01,655 - INFO # Dividing        78125 by 15625...
2018-06-03 18:00:01,657 - INFO # Dividing        78126 by 15625...
2018-06-03 18:00:01,688 - INFO # Dividing        65535 by 15625...
2018-06-03 18:00:01,755 - INFO # Dividing        64512 by 15625...
2018-06-03 18:00:01,757 - INFO # Dividing   4294967295 by 15625...
2018-06-03 18:00:01,955 - INFO # Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing .Dividing 0 by (15625/512)...
2018-06-03 18:00:01,957 - INFO # Dividing 1 by (15625/512)...
2018-06-03 18:00:01,989 - INFO # Dividing 10 by (15625/512)...
2018-06-03 18:00:02,055 - INFO # Dividing 32 by (15625/512)...
2018-06-03 18:00:02,055 - INFO # 
2018-06-03 18:00:02,154 - INFO # div_tests.test_div_u32_by_15625div512 (tests/unittests/tests-div/tests-div.c 94) exp 1 was 0
2018-06-03 18:00:02,157 - INFO # .Dividing 0 by (15625/512)...
2018-06-03 18:00:02,188 - INFO # Dividing 1 by (15625/512)...
2018-06-03 18:00:02,254 - INFO # Dividing 10 by (15625/512)...
2018-06-03 18:00:02,257 - INFO # Dividing 32 by (15625/512)...
2018-06-03 18:00:02,257 - INFO # 
2018-06-03 18:00:02,356 - INFO # div_tests.test_div_u64_by_15625div512 (tests/unittests/tests-div/tests-div.c 121) exp 1 was 0
2018-06-03 18:00:02,387 - INFO # .Dividing 0 by 1000000...
2018-06-03 18:00:02,390 - INFO # Dividing 1 by 1000000...
2018-06-03 18:00:02,454 - INFO # Dividing 10 by 1000000...
2018-06-03 18:00:02,454 - INFO # Dividing 32 by 1000000...
2018-06-03 18:00:02,488 - INFO # Dividing 15625 by 1000000...
2018-06-03 18:00:02,555 - INFO # Dividing 78125 by 1000000...
2018-06-03 18:00:02,556 - INFO # Dividing 78126 by 1000000...
2018-06-03 18:00:02,587 - INFO # Dividing 65535 by 1000000...
2018-06-03 18:00:02,653 - INFO # Dividing 64512 by 1000000...
2018-06-03 18:00:02,654 - INFO # Dividing 4294967295 by 1000000...
2018-06-03 18:00:02,688 - INFO # Dividing Dividing Dividing Dividing Dividing Dividing 
2018-06-03 18:00:02,789 - INFO # div_tests.test_div_u64_by_1000000 (tests/unittests/tests-div/tests-div.c 111) exp 16384 was 16383
2018-06-03 18:00:02,790 - INFO # 
2018-06-03 18:00:02,888 - INFO # run 4 failures 3
/exit
2018-06-03 18:00:06,520 - INFO # Exiting Pyterm
[liveuser@localhost-live unittests]$

@ZetaR60
Copy link
Contributor Author

ZetaR60 commented Jun 4, 2018

I have been tinkering with this a bit, and I cannot get it to come out exactly like what unittests expects. I was hoping that this wouldn't force me to implement floats, but there may be a loss in precision due to the general nature of the calculation method.

@jnohlgard
Copy link
Member

Interesting improvement. Did you compare the computed inverses against the old hard coded magic numbers?

@ZetaR60
Copy link
Contributor Author

ZetaR60 commented Jun 5, 2018

Now tested and working properly via tests/unittests (tests-div).

Comparison between the magic numbers (the new ones are a single bitshift from the old):

New 32-bit (2^n / 15625): 0x8637bd06
10000110001101111011110100000110

Old 32-bit (2^n / 15625): 0x431bde83
01000011000110111101111010000011

New 64-bit  (2^n / 15625): 0x8637bd05af6c69b6
1000011000110111101111010000010110101111011011000110100110110110

Old 64-bit  (2^n / 15625): 0x431bde82d7b634db
0100001100011011110111101000001011010111101101100011010011011011

Test results:

[liveuser@localhost-live unittests]$ make BOARD=mega-xplained test
./tests/01-run.py
/home/liveuser/Desktop/RIOT_div_magic/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "9600"
No handlers could be found for logger "root"
2018-06-05 19:14:51,990 - INFO # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
2018-06-05 19:14:52,995 - INFO # 

2018-06-05 19:14:53,007 - INFO # main(): This is RIOT! (Version: 2018.07-devel-465-g62099-localhost-live-RIOT_div_magic)
2018-06-05 19:14:53,009 - INFO # .Dividing            0 by 15625...
2018-06-05 19:14:53,009 - INFO # Dividing            1 by 15625...
2018-06-05 19:14:53,062 - INFO # Dividing           10 by 15625...
2018-06-05 19:14:53,063 - INFO # Dividing           32 by 15625...
2018-06-05 19:14:53,161 - INFO # Dividing        15625 by 15625...
2018-06-05 19:14:53,163 - INFO # Dividing        78125 by 15625...
2018-06-05 19:14:53,194 - INFO # Dividing        78126 by 15625...
2018-06-05 19:14:53,262 - INFO # Dividing        65535 by 15625...
2018-06-05 19:14:53,263 - INFO # Dividing        64512 by 15625...
2018-06-05 19:14:53,293 - INFO # Dividing   4294967295 by 15625...
2018-06-05 19:14:53,469 - INFO # Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing .Dividing 0 by (15625/512)...
2018-06-05 19:14:53,494 - INFO # Dividing 1 by (15625/512)...
2018-06-05 19:14:53,560 - INFO # Dividing 10 by (15625/512)...
2018-06-05 19:14:53,561 - INFO # Dividing 32 by (15625/512)...
2018-06-05 19:14:53,595 - INFO # Dividing 15625 by (15625/512)...
2018-06-05 19:14:53,660 - INFO # Dividing 78125 by (15625/512)...
2018-06-05 19:14:53,661 - INFO # Dividing 78126 by (15625/512)...
2018-06-05 19:14:53,759 - INFO # Dividing 65535 by (15625/512)...
2018-06-05 19:14:53,760 - INFO # Dividing 64512 by (15625/512)...
2018-06-05 19:14:53,794 - INFO # Dividing 4294967295 by (15625/512)...
2018-06-05 19:14:53,860 - INFO # .Dividing 0 by (15625/512)...
2018-06-05 19:14:53,861 - INFO # Dividing 1 by (15625/512)...
2018-06-05 19:14:53,893 - INFO # Dividing 10 by (15625/512)...
2018-06-05 19:14:53,959 - INFO # Dividing 32 by (15625/512)...
2018-06-05 19:14:53,960 - INFO # Dividing 15625 by (15625/512)...
2018-06-05 19:14:53,993 - INFO # Dividing 78125 by (15625/512)...
2018-06-05 19:14:54,059 - INFO # Dividing 78126 by (15625/512)...
2018-06-05 19:14:54,061 - INFO # Dividing 65535 by (15625/512)...
2018-06-05 19:14:54,093 - INFO # Dividing 64512 by (15625/512)...
2018-06-05 19:14:54,161 - INFO # Dividing 4294967295 by (15625/512)...
2018-06-05 19:14:54,293 - INFO # Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing .Dividing 0 by 1000000...
2018-06-05 19:14:54,362 - INFO # Dividing 1 by 1000000...
2018-06-05 19:14:54,363 - INFO # Dividing 10 by 1000000...
2018-06-05 19:14:54,364 - INFO # Dividing 32 by 1000000...
2018-06-05 19:14:54,394 - INFO # Dividing 15625 by 1000000...
2018-06-05 19:14:54,462 - INFO # Dividing 78125 by 1000000...
2018-06-05 19:14:54,463 - INFO # Dividing 78126 by 1000000...
2018-06-05 19:14:54,494 - INFO # Dividing 65535 by 1000000...
2018-06-05 19:14:54,563 - INFO # Dividing 64512 by 1000000...
2018-06-05 19:14:54,566 - INFO # Dividing 4294967295 by 1000000...
2018-06-05 19:14:54,793 - INFO # Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing Dividing 
2018-06-05 19:14:54,795 - INFO # OK (4 tests)

[liveuser@localhost-live unittests]$ echo $?
0
Installed compiler toolchains 
-----------------------------
             native gcc: missing
      arm-none-eabi-gcc: missing
                avr-gcc: avr-gcc (Fedora 7.2.0-1.fc27) 7.2.0
       mips-mti-elf-gcc: missing
             msp430-gcc: missing
   riscv-none-embed-gcc: missing
                  clang: missing

Installed compiler libs
-----------------------
   arm-none-eabi-newlib: missing
    mips-mti-elf-newlib: missing
riscv-none-embed-newlib: missing
               avr-libc: "2.0.0" ("20150208")

Installed development tools
---------------------------
                  cmake: cmake version 3.11.0
               cppcheck: missing
                doxygen: missing
                 flake8: missing
                    git: git version 2.14.2
             coccinelle: missing

@ZetaR60
Copy link
Contributor Author

ZetaR60 commented Jun 7, 2018

Added __attribute__((optimize("merge-all-constants"))), which should make this a bit safer to use with const variables held in ROM, which otherwise would prevent the magic number calculations from being optimized away.

@jnohlgard
Copy link
Member

@ZetaR60 perhaps we can combine the frac and div modules and get the performance of div and the runtime configurable constants of frac?

@ZetaR60
Copy link
Contributor Author

ZetaR60 commented Jun 7, 2018

@gebart How are you considering doing this combination? Is div still clearly better than frac when frac is used with constant parameters?

I rewrote div primarily because I am working on adding multiple timer support (including RTT/RTC) to xtimer, and it is difficult to convert clocks when things are not all hard-coded. This is still very WIP, but I can create a PR early for it so that it is easier to discuss what the requirements are for clock conversion moving forward.

@ZetaR60
Copy link
Contributor Author

ZetaR60 commented Jun 7, 2018

@gebart What do you think about the best way to interface something like #9308 with div or frac? When writing a function to convert ticks to us the most straightforward way is to have xtimer_dev->params.hz passed to a function. This won't work for div as written, because in #9308 xtimer_dev can change, so hz cannot be replaced with a constant at compile time. My thought was to reference all of the static const xtimer_params_t xtimer_params[] array in the function (see xtimer_params.h), so that it can propagate the constants, but perhaps there is a better way?

@jnohlgard
Copy link
Member

@ZetaR60 for runtime configurable frequencies I would just add a static frac_t inside the module, or a frac_t member in the params struct.

@ZetaR60 ZetaR60 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Jun 14, 2018
@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@stale stale bot closed this Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: stale State: The issue / PR has no activity for >185 days Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants