You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/12/30 17:18:23 UTC

[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request #5125: libc/math: fix log and logf calculations on ARMv7 (and maybe others)

pkarashchenko opened a new pull request #5125:
URL: https://github.com/apache/incubator-nuttx/pull/5125


   ## Summary
   Probably this is a bug of a GCC, but on AMRv7 the code `if (relax_factor > 1)` generates `bne.n` instruction in `relax_factor` is `int`. Few lines above `relax_factor *= LOG_RELAX_MULTIPLIER;` is done without overflow check hence at some moment overflow occurs and `relax_factor` becomes a zero and condition `if (relax_factor > 1)` becomes always evaluated to `true` hence `epsilon` becomes zero always and `log` call never returns.
   
   Probably this is not the best way to fix the bug (The best way is to report it to GCC), but this change allows to get correct behavior of `log` and `logf` for ARMv7 based MCUs
   
   ## Impact
   Native math lib users
   
   ## Testing
   Tested on SAME70 based device.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5125: libc/math: fix log and logf calculations on ARMv7 (and maybe others)

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5125:
URL: https://github.com/apache/incubator-nuttx/pull/5125#issuecomment-1003199336


   Compiler folks recommend one of the possible solutions:
   1. Use `-fwrapv` flag
   2. Switch from `int` to `unsigned int`
   3. Use builtin functions for overflow detection
   
   Probably switching to `unsigned int` is the most lightweight option. I will redo the change


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5125: libc/math: fix log and logf calculations on ARMv7 (and maybe others)

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5125:
URL: https://github.com/apache/incubator-nuttx/pull/5125#issuecomment-1003195261


   Created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103870 to track related issue on GCC side


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko edited a comment on pull request #5125: libc/math: fix log and logf calculations on ARMv7 (and maybe others)

Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5125:
URL: https://github.com/apache/incubator-nuttx/pull/5125#issuecomment-1003199336


   Compiler folks recommend one of the possible solutions:
   1. Use `-fwrapv` flag
   2. Switch from `int` to `unsigned int`
   3. Use builtin functions for overflow detection
   
   Probably switching to `unsigned int` is the most lightweight option, but it still require some overflow protection code.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5125: libc/math: fix log and logf calculations on ARMv7 (and maybe others)

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5125:
URL: https://github.com/apache/incubator-nuttx/pull/5125


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5125: libc/math: fix log and logf calculations on ARMv7 (and maybe others)

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5125:
URL: https://github.com/apache/incubator-nuttx/pull/5125#issuecomment-1003120927


   @patacongo I'm not sure who is the author of math lib code. Will appreciate your input on this change


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko edited a comment on pull request #5125: libc/math: fix log and logf calculations on ARMv7 (and maybe others)

Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5125:
URL: https://github.com/apache/incubator-nuttx/pull/5125#issuecomment-1003199336


   Compiler folks recommend one of the possible solutions:
   1. Use `-fwrapv` flag
   2. Switch from `int` to `unsigned int`
   3. Use builtin functions for overflow detection
   
   Probably switching to `unsigned int` is the most lightweight option, but it still require some overflow protection code.
   I can redo the change to switch to `unsigned int` + overflow check that will be more lightweight for architectures that use `softfp`. However proposed change seems to be more robust and reliable even if it consumes some more cycles. I will wait for some additional feedback in comments


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org