Implement first version of HFlowEngine, a framework for performing global flow-sensitive analyses.

Implement first version of HFlowEngine, a framework for performing global flow-sensitive analyses.

BUG=
R=mstarzinger@chromium.org

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

[titzer, 2013-10-10 09:35:41]

On 2013/09/30 11:51:38, titzer wrote:

Friendly ping

[Michael Starzinger, 2013-10-10 13:00:44]

LGTM with a couple of minor comments.

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h
File src/hydrogen-flow-engine.h (right):

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
src/hydrogen-flow-engine.h:48: // do nothing.
If Disabled() returns true above, none of these methods should actually be
called. Could be add an UNREACHABLE macro instead of the "do nothing" comment?

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
src/hydrogen-flow-engine.h:50: inline void Process(HValue* value, Zone* zone) {
I think the value here should be an HInstruction as Phis cannot be processed as
part of a state IIUC.

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
src/hydrogen-flow-engine.h:51: // do nothing.
Likewise.

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
src/hydrogen-flow-engine.h:54: // do nothing.
Likewise.

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
src/hydrogen-flow-engine.h:65: inline NoState* Process(HValue* value, Zone*
zone) {
I think the value here should be an HInstruction as Phis cannot be processed as
part of a state IIUC.

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
src/hydrogen-flow-engine.h:109: if (new_state != state) SetStateAt(block,
new_state);
Should we really update the state for each block here? IIUC the block_states
list keeps the state at the block entry for each block, so we shouldn't update
it as we go through the instructions inside a block.

https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
src/hydrogen-flow-engine.h:139: if (new_state != state) SetStateAt(block,
new_state);
See comment above.

[titzer, 2013-10-11 12:59:52]

On 2013/10/10 13:00:44, Michael Starzinger wrote:
> LGTM with a couple of minor comments.
> 
> https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h
> File src/hydrogen-flow-engine.h (right):
> 
>
https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
> src/hydrogen-flow-engine.h:48: // do nothing.
> If Disabled() returns true above, none of these methods should actually be
> called. Could be add an UNREACHABLE macro instead of the "do nothing" comment?

In such a situation it's harmless, so I usually wouldn't want to hardwire it to
break.

> 
>
https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
> src/hydrogen-flow-engine.h:50: inline void Process(HValue* value, Zone* zone)
{
> I think the value here should be an HInstruction as Phis cannot be processed
as
> part of a state IIUC.
> 
>
https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
> src/hydrogen-flow-engine.h:51: // do nothing.
> Likewise.
> 
>
https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
> src/hydrogen-flow-engine.h:54: // do nothing.
> Likewise.
> 
>
https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
> src/hydrogen-flow-engine.h:65: inline NoState* Process(HValue* value, Zone*
> zone) {
> I think the value here should be an HInstruction as Phis cannot be processed
as
> part of a state IIUC.
> 

Done.

>
https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
> src/hydrogen-flow-engine.h:109: if (new_state != state) SetStateAt(block,
> new_state);
> Should we really update the state for each block here? IIUC the block_states
> list keeps the state at the block entry for each block, so we shouldn't update
> it as we go through the instructions inside a block.
> 

Good point. Done.

>
https://codereview.chromium.org/25254002/diff/5001/src/hydrogen-flow-engine.h...
> src/hydrogen-flow-engine.h:139: if (new_state != state) SetStateAt(block,
> new_state);
> See comment above.

Yep.

[titzer, 2013-10-11 13:00:48]

Committed patchset #4 manually as r17159 (presubmit successful).