HasNonSmiUse should ignore tagged uses

HasNonSmiUse should ignore tagged uses

R=verwaest@chromium.org

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

[weiliang.lin2, 2013-07-30 04:17:21]

HReturn's RequiredInputRepresentation is Tagged which could affect the SMI to be
Integer32

[Toon Verwaest, 2013-07-30 07:28:36]

Nice catch. However, I'd always ignore tagged uses, not just in the case of
return.

[weiliang.lin2, 2013-07-30 07:50:18]

On 2013/07/30 07:28:36, Toon Verwaest wrote:
> Nice catch. However, I'd always ignore tagged uses, not just in the case of
> return.

Do you mean using the following statement instead?
 if (!use_rep.IsNone() && !use_rep.IsSmi() && !use_rep.IsTagged())) return true;

[Toon Verwaest, 2013-07-30 08:11:05]

Yes.

[weiliang.lin2, 2013-07-30 08:22:15]

On 2013/07/30 08:11:05, Toon Verwaest wrote:
> Yes.

Address and also update the title. Thanks

[Toon Verwaest, 2013-07-30 08:32:05]

Lgtm with nit.

https://codereview.chromium.org/21105006/diff/8001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/21105006/diff/8001/src/hydrogen-instructions....
src/hydrogen-instructions.cc:4379: && !use_rep.IsTagged()) return true;
Nit: use new lines for each of the three cases, put the && at the end of the
line rather than the start, and put {} around the true case.
Otherwise you could also just check for Integer32 and Double instead.

[weiliang.lin2, 2013-07-30 08:37:12]

On 2013/07/30 08:32:05, Toon Verwaest wrote:
> Lgtm with nit.
> 
>
https://codereview.chromium.org/21105006/diff/8001/src/hydrogen-instructions.cc
> File src/hydrogen-instructions.cc (right):
> 
>
https://codereview.chromium.org/21105006/diff/8001/src/hydrogen-instructions....
> src/hydrogen-instructions.cc:4379: && !use_rep.IsTagged()) return true;
> Nit: use new lines for each of the three cases, put the && at the end of the
> line rather than the start, and put {} around the true case.
> Otherwise you could also just check for Integer32 and Double instead.

Thanks for comments
Please review the new patch set

[Toon Verwaest, 2013-07-30 09:00:00]

lgtm.

[Toon Verwaest, 2013-07-30 09:06:47]

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