refactor test instruction on ia32

refactor test instruction on ia32

BUG=

Committed: http://code.google.com/p/v8/source/detail?r=16901

[weiliang.lin2, 2013-09-09 14:49:22]

Make test(mem, imm) employ test_b too.

[Jakob Kummerow, 2013-09-09 19:16:51]

Patch set 1 LGTM with comment.

Does your second comment refer to another patch set that you forgot to upload?

https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc
File src/ia32/assembler-ia32.cc (right):

https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc#ne...
src/ia32/assembler-ia32.cc:1170: }
I think we may want to keep the "op.is_reg_only()" path (in addition to the
is_uint8(imm.x_) check).

[weiliang.lin2, 2013-09-10 03:09:41]

https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc
File src/ia32/assembler-ia32.cc (right):

https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc#ne...
src/ia32/assembler-ia32.cc:1170: }
On 2013/09/09 19:16:51, Jakob wrote:
> I think we may want to keep the "op.is_reg_only()" path (in addition to the
> is_uint8(imm.x_) check).

Actually op.is_reg_only() is handled in test_b functions.

[Jakob Kummerow, 2013-09-11 11:04:06]

On 2013/09/10 03:09:41, weiliang.lin2 wrote:
> https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc
> File src/ia32/assembler-ia32.cc (right):
> 
>
https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc#ne...
> src/ia32/assembler-ia32.cc:1170: }
> On 2013/09/09 19:16:51, Jakob wrote:
> > I think we may want to keep the "op.is_reg_only()" path (in addition to the
> > is_uint8(imm.x_) check).
> 
> Actually op.is_reg_only() is handled in test_b functions.

Yes, it is, but the old code has a reg_only path for non-int8 immediates, which
the new code does not. I haven't audited all call sites to see if we actually
make use of this, but since it's easy to support, it would be nice to keep
supporting it.

[weiliang.lin2, 2013-09-11 12:56:18]

On 2013/09/11 11:04:06, Jakob wrote:
> On 2013/09/10 03:09:41, weiliang.lin2 wrote:
> > https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc
> > File src/ia32/assembler-ia32.cc (right):
> > 
> >
>
https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc#ne...
> > src/ia32/assembler-ia32.cc:1170: }
> > On 2013/09/09 19:16:51, Jakob wrote:
> > > I think we may want to keep the "op.is_reg_only()" path (in addition to
the
> > > is_uint8(imm.x_) check).
> > 
> > Actually op.is_reg_only() is handled in test_b functions.
> 
> Yes, it is, but the old code has a reg_only path for non-int8 immediates,
which
> the new code does not. I haven't audited all call sites to see if we actually
> make use of this, but since it's easy to support, it would be nice to keep
> supporting it.

Yes, it is. reg_only path should be kept for non-int8 immediate. Thanks for
remind.:)

[weiliang.lin2, 2013-09-23 13:25:50]

On 2013/09/11 12:56:18, weiliang.lin2 wrote:
> On 2013/09/11 11:04:06, Jakob wrote:
> > On 2013/09/10 03:09:41, weiliang.lin2 wrote:
> > > https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc
> > > File src/ia32/assembler-ia32.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/23679007/diff/1/src/ia32/assembler-ia32.cc#ne...
> > > src/ia32/assembler-ia32.cc:1170: }
> > > On 2013/09/09 19:16:51, Jakob wrote:
> > > > I think we may want to keep the "op.is_reg_only()" path (in addition to
> the
> > > > is_uint8(imm.x_) check).
> > > 
> > > Actually op.is_reg_only() is handled in test_b functions.
> > 
> > Yes, it is, but the old code has a reg_only path for non-int8 immediates,
> which
> > the new code does not. I haven't audited all call sites to see if we
actually
> > make use of this, but since it's easy to support, it would be nice to keep
> > supporting it.
> 
> Yes, it is. reg_only path should be kept for non-int8 immediate. Thanks for
> remind.:)

Update to master

[Jakob Kummerow, 2013-09-23 16:58:17]

Thanks, landed as r16901.