503427 - jstracer.cpp creates ill-typed LIR: i2f (i2f (arg)) and variants thereof

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Julian Seward [:jseward]]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: 

TM branch, x86-linux build.  Basically jstracer.cpp creates 
LIR_i2f applied to non-int32 args; 64-bit float and quad args
have been observed.

The reason this doesn't cause havoc is because, in all such cases,
the outer (or, in general, ill-typed) application of i2f is dead,
and so DeadCodeFilter later in the pipeline removes it.

I have no idea if this is merely a harmless bug or whether it
signifies something more serious.  However, it would be good to
remove it, since it produces what amount to false-positives in 
in-progress work on LIR typechecking.

How to repro: take a debug build of jsshell, do:

TMFLAGS=minimal,readlir ./BuildD/js -j ./trace-test.js

and search for "i2f 0:0" in the output.  This is i2f being
applied to a 64-bit constant, when the arg should be 32-bit int.

Finding an easily reproduced case for i2f (i2f (arg)) is not
so easy; still in progress.


Reproducible: Always

Comment 1

[Robert Sayre]

Gross. Nice catch.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: test case

This happens when a loop wants a double at the edge, but there's a promoteInt instead. We insert i2f, despite the fact that a promoteInt is always a quad in LIR. This works because writeBack() will ::demote and strip away the redundant i2f, and the erroneous i2f will die.

Attached is a trace-tests case which emits such weird LIR.

This is a lot easier to fix against the patch in bug 497948 but I think I can hack it into the old code as well.

Comment 3

[Julian Seward [:jseward]]

(In reply to comment #2)
> test case

Yep:

xxx     0xf7f982d0 i2f3 = i2f add1
[..]
xxx     0xf7f98bb0 i2f4 = i2f i2f3
[..]
LirChecker: for argument 1 of the node
   i2f4 = i2f i2f3
   type error: can't reconcile Flt64 with Int32

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: hacky fix

Here is a hacky fix.  I added an assert which did not hit in trace-tests.

Comment 5

[Andreas Gal :gal]

Any better idea how to fix this?

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fixes bug

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/e8a6a3216b05

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/e8a6a3216b05

Comment 9

[Damon Sicore (:damons)]

Not blocking per danderson.

Comment 10

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/925e39ec4508