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

Improve Comments for Pairwise Multiplication Optimization #5524

Closed

Conversation

xu-shawn
Copy link
Contributor

@xu-shawn xu-shawn commented Jul 30, 2024

Inspired by #5522, better explains why we use (127 * 2). Feel free to point out any mistakes and errors, as this is my first time reading this part of the code.

Thanks to @FauziAkram for raising awareness and @Disservin for formatting suggestions.

no functional change

@xu-shawn xu-shawn force-pushed the pairwise-mul-comments branch 8 times, most recently from 59a4440 to 2c62951 Compare July 30, 2024 22:47
@Disservin Disservin added to be merged Will be merged shortly and removed to be merged Will be merged shortly labels Aug 1, 2024
// to the left by 1, so we compensate by shifting less before
// the multiplication.

constexpr int shift =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually takes more lines than before.

Copy link
Member

@Disservin Disservin Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does but with the old way, the comment was intended wrongly by the formatter :/

@vondele vondele added the to be merged Will be merged shortly label Aug 20, 2024
@vondele vondele closed this in bc80ece Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants