[WIP] Convert drfti1 to a safe version #63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@jmvalin if you have time could you help him? :) |
lu-zero
reviewed
Mar 3, 2020
I am still not satisfied with this, but at least it can be a decent starting point for improvements. My points are: 1. Variable names are still unintelligible, mainly because I do not have a clear vision of the algorithm. Maybe someone better than me can be able to grasp the meaning of each variable and to git them a decent name. 2. The first loop is still unidiomatic. Everything I thought involves an additional check to exit the outer loop, and at the current state (no benchmarks) I am not sure about the best approch. Nevertheless, cast checks that were not present in the original code should be helpful to find strange edge cases, and they should have a negligible cost due to the cpu branch predictor.
dodomorandi
force-pushed
the
safe-drfti1
branch
from
ea19cf4
to
0c3a47e
Compare
dodomorandi
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am still not satisfied with this, but at least it can be a decent
starting point for improvements. My points are:
a clear vision of the algorithm. Maybe someone better than me can be
able to grasp the meaning of each variable and to git them a decent
name.
additional check to exit the outer loop, and at the current state (no
benchmarks) I am not sure about the best approch.
Nevertheless, cast checks that were not present in the original code
should be helpful to find strange edge cases, and they should have a
negligible cost due to the cpu branch predictor.