Skip to main content
Code Review

Return to Answer

added 4 characters in body
Source Link
Davislor
  • 9.1k
  • 19
  • 39

Do You Really Want Two’s-Complement?

The main motivation for using it in the CPU is that, for fixed-width unsigned types, the CPU can use the same logic for signed and unsigned addition and subtraction. However, here, you don’t want to do both signed and unsigned addition and subtraction. You end up making subtraction more complex by copying and inverting the second operand, and likewise the other arithmetic.

The classic solution is therefore to use a sign-and-magnitude representation for this purpose.

Some of the Individual Functions Can be Optimized

Some of the Individual Functions Can be Optimized

Do You Really Want Two’s-Complement?

The main motivation for using it in the CPU is that, for fixed-width unsigned types, the CPU can use the same logic for signed and unsigned addition and subtraction. However, here, you don’t want to do both signed and unsigned addition and subtraction. You end up making subtraction more complex by copying and inverting the second operand, and likewise the other arithmetic.

The classic solution is therefore to use a sign-and-magnitude representation for this purpose.

Some of the Individual Functions Can be Optimized

added 4 characters in body
Source Link
Davislor
  • 9.1k
  • 19
  • 39

But second, bitwise and where one of the operands is 0 will always return 0, so you could have done the &= loop only up to the lesser of the two operands’ sizes, and then either zero-extendextended the left operand or zerozeroed out its remaining words.

Naming ConventionMinor Cosmetic Suggetions

Either PascalCase or snake_case is much more common for class names than Whatever_This_Is. I’d recommend you pick one of the first two.

Avoid misleading whitespace such as while (i --> 0). That one does kind of work, but --> is not an operator, and is easy to confuse for ->, which is.

But second, bitwise and where one of the operands is 0 will always return 0, so you could have done the &= loop only up to the lesser of the two operands’ sizes, and then either zero-extend the left operand or zero out its remaining words.

Naming Convention

Either PascalCase or snake_case is much more common for class names than Whatever_This_Is. I’d recommend you pick one of the first two.

But second, bitwise and where one of the operands is 0 will always return 0, so you could have done the &= loop only up to the lesser of the two operands’ sizes, and then either zero-extended the left operand or zeroed out its remaining words.

Minor Cosmetic Suggetions

Either PascalCase or snake_case is much more common for class names than Whatever_This_Is. I’d recommend you pick one of the first two.

Avoid misleading whitespace such as while (i --> 0). That one does kind of work, but --> is not an operator, and is easy to confuse for ->, which is.

Source Link
Davislor
  • 9.1k
  • 19
  • 39

Does this Need to be a Template Class?

You tell us that it can operate on vectors of any unsigned integral type, but is there any use case other than the native word size as an element type? (Which is normally size_t, although maybe you want to write it as using word_t = std::size_t just in case you need to define it differently for some weird target.)

You don’t actually need a double-width word to test for unsigned carry, because you can just compare the result to either operand. For multiplication, the only completely standard and portable way would be to convert 32-bit operands to unsigned long long so that all the bits of the product will fit, but some architectures have extensions you can use to do this faster.

If you really do need the template parameter, you can, as an alternative to the static_assert statements you have now, specify it as the C++20 concept template<std::unsigned_integral N>. It’s also one of several surprising names in the code. I would have expected a N template parameter to be an integral constant, not an integral type. If you do keep it, consider a name like Word.

Use a Spaceship Operator

You could use the code from your compare function to implement <=> and cut down on a lot of the boilerplate for comparisons. You probably still want operator== and operator!= declared separately, but I believe those can just be default.

Follow the Rule of Five

Or actually, six. This type wraps a std::vector, which is much more efficient to move than to copy, so it should have a copy constructor, a move constructor (which can both be default) and a swap operation.

Rvalue Reference Overloads Would be More Efficient

Currently, all or nearly all the operators that return temporaries make a copy of the first operand and update that in place. However, if either operand is moveable (Binary_Register&& instead of const Binary_Register&), you can optimize out the copy. Since nearly all the operations are commutative, modulo an inversion here or there, you can do this if either operand is moveable.

The gain is especially noticeable in an expression such as a = w + x + y + z;. The current operation would need to make four copies, but adding an overload for && on the left operand would reduce that to one: w would need to be copied, in the first addition, but then every other sum would have an xvalue (the temporary returned by the previous addition) as its left operand, so it could keep updating that in place, then move the result to a.

Some of the Individual Functions Can be Optimized

To give just one example, &= pads the destination operand to the length of the source with a loop that calls push_back(0) on the vector of words, then does element-wise &=.

First, when you know the total size, it’s much more efficient to pad with zeroes by calling resize once than by calling push_back repeatedly.

But second, bitwise and where one of the operands is 0 will always return 0, so you could have done the &= loop only up to the lesser of the two operands’ sizes, and then either zero-extend the left operand or zero out its remaining words.

Naming Convention

Either PascalCase or snake_case is much more common for class names than Whatever_This_Is. I’d recommend you pick one of the first two.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /