Fix fixed-point vcvt_f64_s32 immediate value encoding

Fix fixed-point vcvt_f64_s32 immediate value encoding

The (32 - fraction_bits) value should be encoded so that the least
significant bit is set to bit 5 and the four next bits to bits 0-3. Fix
the previously incorrect encoding. This bug did not cause behavioral
issues before, since in existing uses of the function the order of the
bits in the immediate value does not matter, as they are all 1.

BUG=3256
LOG=N
R=ulan@chromium.org

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

[oetuaho-nv, 2014-04-03 17:59:55]

Spun off bugfix part from https://codereview.chromium.org/222403002/ since the
optimization is likely to take a different route.

[rmcilroy, 2014-04-03 18:59:10]

+ulan

Seems reasonable to me, but adding Ulan to check as well.

https://codereview.chromium.org/223623003/diff/1/src/arm/disasm-arm.cc
File src/arm/disasm-arm.cc (right):

https://codereview.chromium.org/223623003/diff/1/src/arm/disasm-arm.cc#newcod...
src/arm/disasm-arm.cc:1275: int fraction_bits = 32 - (instr->Bit(5) |
(instr->Bits(3, 0) << 1));
nit - please rewrite as:
(instr->Bits(3, 0) << 1) | (instr->Bit(5)
for clarity.

https://codereview.chromium.org/223623003/diff/1/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/223623003/diff/1/src/arm/simulator-arm.cc#new...
src/arm/simulator-arm.cc:2939: int fraction_bits = 32 - (instr->Bit(5) |
(instr->Bits(3, 0) << 1));
ditto

[ulan, 2014-04-04 09:30:14]

On 2014/04/03 18:59:10, rmcilroy wrote:
> +ulan
> 
> Seems reasonable to me, but adding Ulan to check as well.
> 
> https://codereview.chromium.org/223623003/diff/1/src/arm/disasm-arm.cc
> File src/arm/disasm-arm.cc (right):
> 
>
https://codereview.chromium.org/223623003/diff/1/src/arm/disasm-arm.cc#newcod...
> src/arm/disasm-arm.cc:1275: int fraction_bits = 32 - (instr->Bit(5) |
> (instr->Bits(3, 0) << 1));
> nit - please rewrite as:
> (instr->Bits(3, 0) << 1) | (instr->Bit(5)
> for clarity.
> 
> https://codereview.chromium.org/223623003/diff/1/src/arm/simulator-arm.cc
> File src/arm/simulator-arm.cc (right):
> 
>
https://codereview.chromium.org/223623003/diff/1/src/arm/simulator-arm.cc#new...
> src/arm/simulator-arm.cc:2939: int fraction_bits = 32 - (instr->Bit(5) |
> (instr->Bits(3, 0) << 1));
> ditto

lgtm if the comments are addressed.

[oetuaho-nv, 2014-04-04 10:17:13]

Patch set 2 addressed the comments, committing.

[oetuaho-nv, 2014-04-04 10:17:17]

The CQ bit was checked by oetuaho@nvidia.com

[commit-bot: I haz the power, 2014-04-04 10:20:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-04 10:20:08]

Commit queue rejected this change because it did not recognize the base URL.
Please commit your change manually.

[oetuaho-nv, 2014-04-04 10:38:45]

Looks like commit bot is rejecting the change. I used instructions from
https://github.com/v8/v8/wiki/Proposing-a-Patch

Are they up to date?

[rmcilroy, 2014-04-04 10:55:39]

On 2014/04/04 10:38:45, oetuaho-nv wrote:
> Looks like commit bot is rejecting the change. I used instructions from
> https://github.com/v8/v8/wiki/Proposing-a-Patch
> 
> Are they up to date?

The commit queue doesn't work for the V8 project, and without committer status
you won't be able to land it manually. I will land this for you.

[rmcilroy, 2014-04-04 11:12:48]

Committed patchset #2 manually as r20508 (presubmit successful).