A64: When truncating to an int, store the result in a 64bit register

A64: When truncating to an int, store the result in a 64bit register

We don't check later one whether the result is 32bit or 64bit, so we end
up using an undefined value otherwise.

BUG=none
R=ulan@chromium.org, rmcilroy@chromium.org, rodolph.perfetta@arm.com
TEST=mjsunit/sin-cos.js
LOG=n

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

[ulan, 2014-02-11 12:52:45]

lgtm

[jochen (gone - plz use gerrit), 2014-02-11 12:56:18]

updated to remove the now dead code from macro assembler and updated tests

PTAL

[rmcilroy, 2014-02-11 12:58:17]

On 2014/02/11 12:56:18, jochen wrote:
> updated to remove the now dead code from macro assembler and updated tests
> 
> PTAL

lgtm. 

As an aside, my CL to port https://codereview.chromium.org/23129003 to A64 will
remove the SMI option as well and always return the value as an Int32 in the X
register.

[ulan, 2014-02-11 12:58:28]

lgtm

[jochen (gone - plz use gerrit), 2014-02-11 12:58:31]

Committed patchset #3 manually as r19279 (presubmit successful).

[jbramley, 2014-02-11 12:58:50]

https://codereview.chromium.org/131333011/diff/40001/src/a64/lithium-codegen-...
File src/a64/lithium-codegen-a64.cc (right):

https://codereview.chromium.org/131333011/diff/40001/src/a64/lithium-codegen-...
src/a64/lithium-codegen-a64.cc:5496: : MacroAssembler::INT32);
Our convention for Lithium instructions is to leave the top word of an int32
value undefined. This allows some minor efficiency improvements. We make similar
assumptions elsewhere too.

Have you found a case where the top 32 bits are read? If an A64 Lithium
instruction takes an Integer32 representation, it shouldn't read the top word.

[jochen (gone - plz use gerrit), 2014-02-11 13:01:02]

On 2014/02/11 12:58:50, jbramley wrote:
>
https://codereview.chromium.org/131333011/diff/40001/src/a64/lithium-codegen-...
> File src/a64/lithium-codegen-a64.cc (right):
> 
>
https://codereview.chromium.org/131333011/diff/40001/src/a64/lithium-codegen-...
> src/a64/lithium-codegen-a64.cc:5496: : MacroAssembler::INT32);
> Our convention for Lithium instructions is to leave the top word of an int32
> value undefined. This allows some minor efficiency improvements. We make
similar
> assumptions elsewhere too.
> 
> Have you found a case where the top 32 bits are read? If an A64 Lithium
> instruction takes an Integer32 representation, it shouldn't read the top word.

BoundsCheck compares the X registers (and thus fails) in mjsunit/sin-cos.js

[Rodolph Perfetta, 2014-02-11 13:02:04]

not lgtm.

As Jacob explained the top bits shouldn't be read, if you found a case where it
is, then there is another bug lurking and you are just hiding it.

[jochen (gone - plz use gerrit), 2014-02-11 13:05:16]

On 2014/02/11 13:02:04, Rodolph Perfetta wrote:
> not lgtm.
> 
> As Jacob explained the top bits shouldn't be read, if you found a case where
it
> is, then there is another bug lurking and you are just hiding it.

As mentioned above, LCodeGen::DoBoundsCheck compares x registers with each
other.

So a better fix was to use ToRegister32 in DoBoundsCheck?

[jbramley, 2014-02-11 13:24:43]

On 2014/02/11 13:05:16, jochen wrote:
> On 2014/02/11 13:02:04, Rodolph Perfetta wrote:
> > not lgtm.
> > 
> > As Jacob explained the top bits shouldn't be read, if you found a case where
> it
> > is, then there is another bug lurking and you are just hiding it.
> 
> As mentioned above, LCodeGen::DoBoundsCheck compares x registers with each
> other.
> 
> So a better fix was to use ToRegister32 in DoBoundsCheck?

For the Integer32 representation, yes, though smi representations need a 64-bit
comparison, so you have to use the representation to select between ToRegister
and ToRegister32. The constant-index case already does this, though it gets it
wrong because it still compares an X-sized 'length' with the constant.

MulConstIS is an example of how this is handled elsewhere. We sometimes also
split instructions for different representations (like DoAddI and DoAddS).

[jochen (gone - plz use gerrit), 2014-02-11 13:40:38]

On 2014/02/11 13:24:43, jbramley wrote:
> On 2014/02/11 13:05:16, jochen wrote:
> > On 2014/02/11 13:02:04, Rodolph Perfetta wrote:
> > > not lgtm.
> > > 
> > > As Jacob explained the top bits shouldn't be read, if you found a case
where
> > it
> > > is, then there is another bug lurking and you are just hiding it.
> > 
> > As mentioned above, LCodeGen::DoBoundsCheck compares x registers with each
> > other.
> > 
> > So a better fix was to use ToRegister32 in DoBoundsCheck?
> 
> For the Integer32 representation, yes, though smi representations need a
64-bit
> comparison, so you have to use the representation to select between ToRegister
> and ToRegister32. The constant-index case already does this, though it gets it
> wrong because it still compares an X-sized 'length' with the constant.
> 
> MulConstIS is an example of how this is handled elsewhere. We sometimes also
> split instructions for different representations (like DoAddI and DoAddS).

I guess I'm just unlucky, but the code following the bounds check can't cope
with the clobbered upper 32bit either... filed
https://code.google.com/p/v8/issues/detail?id=3149 to track this.