Fix load-elim bug for branching instructions going to the same block
commit20c4b24ce52f96a59ae7399bad70ede08b16f4bf
authorRick Lavoie <rlavoie@fb.com>
Wed, 12 Apr 2017 01:18:55 +0000 (11 18:18 -0700)
committerHhvm Bot <hhvm-bot@users.noreply.github.com>
Wed, 12 Apr 2017 01:37:48 +0000 (11 18:37 -0700)
tree5c833489d79c99807cf707829df314bd2206f473
parenta25fe91f038144716ae8f49f17b664db970729d6
Fix load-elim bug for branching instructions going to the same block

Summary:
If a branching instruction has the same block as the next and taken target,
load-elim will always choose the next state to merge into the successor. This
can cause problems if the instruction alters the tracked state and we later
simplify away the branch instruction.

Here's a somewhat constrived example to illustrate the problem:

```
B1:
   (1) t1:Str|InitNull = LdLoc<Str|InitNull,1>
   (2) t2:Str = CheckType<Str> t1:Str|InitNull B2, B2

B2:
   (3) t3:Str|InitNull = LdLoc<Str|InitNull,1>
   (4) DecRef t3:Str|InitNull
```

Load-elim will deduce at (2) that a load from local slot #1 can be substituted
with t2 (along the next branch). When it processes (3), it will choose the next
branch's state to merge. So, it will rewrite (3) to t2 and
copy-propagate. Obtaining:

```
B1:
   (1) t1:Str|InitNull = LdLoc<Str|InitNull,1>
   (2) t2:Str = CheckType<Str> t1:Str|InitNull B2, B2

B2:
   (4) DecRef t2:Str
```

However, the simplifier and DCE now kicks in and optimizes away (2) because both
of its branches jump to the same block, leaving us with just:

```
B1:
   (4) DecRef t2:Str
```

IE, we're dec-reffing a tmp that no longer has a definition.

The fix for all this is use the taken branch's state when an instruction has the
same block for next and taken. In the above example, the taken branch's state
won't include the subsitution rule, so we won't copy propagate t2 before
eliminating it.

Reviewed By: markw65

Differential Revision: D4870758

fbshipit-source-id: 12f3311b63c1be58cb3ba36dd62228c95446ef76
hphp/runtime/vm/jit/check.cpp
hphp/runtime/vm/jit/load-elim.cpp