Generalized division via multiplication.

Generalized division via multiplication.

We can now compute the magic numbers for all combinations of 32bit and
64bit (un)signed multiplications.

R=bmeurer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=23730

[Sven Panne, 2014-09-03 14:31:28]

svenpanne@chromium.org changed reviewers:
+ bmeurer@chromium.org

[Sven Panne, 2014-09-03 14:31:28]

The call sites could be nicer, but I want to land this drop-in replacement first
before doing any more cleanup. Real unit tests can be added later, even
exhausting testing of the 32bit variants is relatively quick when doing
multi-threaded on our workstations.

Somehow I feel that assembler.{h,cc} is not a nice place for these classes,
better suggestions welcome...

[Benedikt Meurer, 2014-09-04 04:20:45]

Hm, I don't think this is the right approach. And didn't we decide that we delay
this for now, because visualization is way more important?

Anyway, here's what I'd suggest:

1. Put the stuff in src/compiler/division-by-constant.{cc,h}
2. Rename to DivisionByConstant, MultiplierAndShift is not very useful name.
3. Add unit tests first, before writing the actual implementation (right now we
only assume that it's correct, at least for the code paths not used in
Crankshaft).
4. I would not even touch the existing code in the MacroAssemblers, but instead
add a TODO, that TruncatingDiv should be removed once Turbofan replaces
Crankshaft to a certain degree (i.e. I don't think this optimization is really
required once all asm.js code runs through Turbofan).
5. Use stuff inside the MachineOperatorReducer, again with tests to cover the
basics. Maybe add stress tests to cover everything.

Thinking more about it, we could even encapsulate everything inside the
MachineOperatorReducer, because it's local to it and still properly test it
(avoids duplication of tests). But then again, we should probably delay working
on this, I dunno.

[Sven Panne, 2014-09-04 12:27:36]

Moved stuff around as discussed offline, added unit tests. PTAL...

[Sven Panne, 2014-09-04 13:15:34]

Patchset #5 (id:80001) has been deleted

[Sven Panne, 2014-09-04 13:15:41]

Patchset #4 (id:60001) has been deleted

[Sven Panne, 2014-09-04 13:16:12]

Patchset #3 (id:40001) has been deleted

[Sven Panne, 2014-09-04 13:16:52]

Patchset #2 (id:20001) has been deleted

[Sven Panne, 2014-09-05 08:20:52]

Patchset #3 (id:120001) has been deleted

[Sven Panne, 2014-09-05 10:29:14]

Patchset #5 (id:180001) has been deleted

[Sven Panne, 2014-09-05 10:29:20]

Patchset #3 (id:140001) has been deleted

[Sven Panne, 2014-09-05 10:29:27]

Patchset #3 (id:160001) has been deleted

[Sven Panne, 2014-09-05 10:31:29]

As discussed offline: Removed the helper functions containing EXPECT_EQ. PTAL...

[Benedikt Meurer, 2014-09-05 11:15:19]

LGTM with comment.

https://codereview.chromium.org/532003004/diff/220001/src/base/division-by-co...
File src/base/division-by-constant-unittest.cc (right):

https://codereview.chromium.org/532003004/diff/220001/src/base/division-by-co...
src/base/division-by-constant-unittest.cc:11: #include "src/base/macros.h"
You should not need this include.

[Sven Panne, 2014-09-05 11:42:29]

On 2014/09/05 11:15:19, Benedikt Meurer wrote:
> LGTM with comment.
> 
>
https://codereview.chromium.org/532003004/diff/220001/src/base/division-by-co...
> File src/base/division-by-constant-unittest.cc (right):
> 
>
https://codereview.chromium.org/532003004/diff/220001/src/base/division-by-co...
> src/base/division-by-constant-unittest.cc:11: #include "src/base/macros.h"
> You should not need this include.

Removed, landing...

[Sven Panne, 2014-09-05 11:49:04]

Committed patchset #6 (id:260001) manually as 23730 (presubmit successful).