Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#9231 closed Bugs (fixed)

Suboptimal assignment

Reported by: Domagoj Šarić Owned by: John Maddock
Milestone: To Be Determined Component: multiprecision
Version: Boost 1.54.0 Severity: Optimization
Keywords: Cc:

Description (last modified by Domagoj Šarić)

Hi, we're using your MP library, with the cpp_int_backend and fixed sized numbers, along with the "Maybe Boost Hash" library to implement lightweight RSA signature verification (previously done with LibTomCrypt+LibTomMath), so first of all thanks for making this possible ;)

Unless otherwise stated, always assume I'm using MSVC10 32bit (with /Oxs /GL /LTCG) with the above use case (fixed sized, unchecked, unsigned cpp_ints)...

Now, onto the issues ;D

I would expect do_assign() to simply evaluate to simply std::memcpy, however it does not:

  • it seems that the this->resize(other.size(), other.size()); call fails to resolve to a nop (check your code)
  • why are you using "checked std::copy" instead of a straigh std::memcpy?

ps. MSVC has a very nice debugger and disassembly window so I'll refrain from posting codegen in any of my tickets.

Change History (7)

comment:1 by Domagoj Šarić, 9 years ago

Component: Nonemultiprecision
Owner: set to John Maddock

comment:2 by John Maddock, 9 years ago

Resolution: fixed
Status: newclosed

(In [86258]) Optimize copying of allocator-free cpp_int's via memcpy. Fix consistency of checks for exponents < 0 in powm. Fixes #9231. Refs #9236.

comment:3 by anonymous, 9 years ago

Thanks... ps. did you check why this->resize(other.size(), other.size()); does not evaluate to a nop (I forgot to add that this happens in powm when it copy-constructs the double_type)_?

comment:4 by Domagoj Šarić, 9 years ago

Description: modified (diff)

comment:5 by John Maddock, 9 years ago

Thanks... ps. did you check why this->resize(other.size(), other.size()); does not evaluate to a nop (I forgot to add that this happens in powm when it copy-constructs the double_type)_?

Sigh.... this is harder to optimize away - it goes via a general N bit to M bit conversion routine (which is already split many different ways, I really don't want to split it up any more or it becomes unmaintainable) so there is a check that the new size requested doesn't exceed the space available. This situation can also happen in calls to resize() from addition/subtraction/multiplication/shift routines, as it has to handle overflow in these cases.

comment:6 by Domagoj Šarić, 9 years ago

MSVC is good at propagating constants (which need not be actual ICEs, but can be resolved as such at LTCG time) so that resize() is receiving two constants there...the fact that the compiler wasn't able to fully eliminate the code (the function call isn't there, it was inlined but not fully eliminated, I could see some branching) smells of something 'weird' being done in the resize() function...don't have time currently to examine the matter further...

comment:7 by John Maddock, 9 years ago

(In [86262]) Use memcpy in more places. Add optimized bitwise operations for unsigned integers. Fixes #9243. Refs #9231.

Note: See TracTickets for help on using tickets.