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

Replace use of LargerType in constructors with ShiftType #83

Open
Pharap opened this issue Mar 25, 2021 · 1 comment
Open

Replace use of LargerType in constructors with ShiftType #83

Pharap opened this issue Mar 25, 2021 · 1 comment
Assignees
Labels
Improvement This change improves the code's behaviour Optimisation This change somehow improves optimisation Patch This change is an unobtrusive change

Comments

@Pharap
Copy link
Owner

Pharap commented Mar 25, 2021

This change would have two potential benefits:

  • The undefined behaviour eminating from << with a negative left hand value would be replaced with the well-defined behaviour of an unsigned type.
  • Theoretically this should generate less code because the code would no longer be using unnecessary excess bytes during the shift operation. Whether this is true in practice would have to be tested - the compiler may already be optimising this.

The code should be functionally equivalent, hence this should be a 'patch' change.

@Pharap Pharap added Patch This change is an unobtrusive change Improvement This change improves the code's behaviour labels Mar 25, 2021
@Pharap Pharap self-assigned this Mar 25, 2021
@Pharap
Copy link
Owner Author

Pharap commented Mar 25, 2021

As a reminder:
static_cast<InternalType>(static_cast<ShiftType>(value) << FractionSize)

@Pharap Pharap added the Optimisation This change somehow improves optimisation label Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement This change improves the code's behaviour Optimisation This change somehow improves optimisation Patch This change is an unobtrusive change
Projects
None yet
Development

No branches or pull requests

1 participant