[wasm] Fold bounds checks during graph building.

[wasm] Fold bounds checks during graph building.

R=ahaas@chromium.org,clemensh@chromium.org
BUG=

Committed: https://crrev.com/aee1824adbcadb97bd7295e87e1e6765997bd8f9
Cr-Commit-Position: refs/heads/master@{#36100}

[Clemens Hammacher, 2016-05-09 08:32:32]

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler.cc
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler....
src/compiler/wasm-compiler.cc:2581:
trap_->AddTrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);
Why not trap_->TrapAlways?

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler....
src/compiler/wasm-compiler.cc:2587: CHECK(effective_size <= kMaxUInt32);
Wouldn't a DCHECK suffice here?

[titzer, 2016-05-09 08:38:17]

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler.cc
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler....
src/compiler/wasm-compiler.cc:2581:
trap_->AddTrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);
On 2016/05/09 08:32:31, Clemens Hammacher wrote:
> Why not trap_->TrapAlways?

Yes, that would be great, but the nodes that will be build after the load needs
effect and control dependencies. A natural choice would be {Dead}, but that
causes problems if it is latent in the graph before scheduling (i.e. it requires
running the dead code elimination phase to clean up the graph). We can come back
to that when we want to run the dead code elimination phase and other
optimizations on the graph before scheduling.

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler....
src/compiler/wasm-compiler.cc:2587: CHECK(effective_size <= kMaxUInt32);
On 2016/05/09 08:32:31, Clemens Hammacher wrote:
> Wouldn't a DCHECK suffice here?

I really, really, don't want to wrap around in production, so being extra
careful here.

[Clemens Hammacher, 2016-05-09 08:47:40]

lgtm

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler.cc
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler....
src/compiler/wasm-compiler.cc:2587: CHECK(effective_size <= kMaxUInt32);
On 2016/05/09 08:38:17, titzer wrote:
> On 2016/05/09 08:32:31, Clemens Hammacher wrote:
> > Wouldn't a DCHECK suffice here?
> 
> I really, really, don't want to wrap around in production, so being extra
> careful here.

It really should not be possible here, since (offset+memsize <= size) here, and
offset+memsize cannot overflow either.
But I see the point.
And maybe the compiler is even smart enough to remove this check in production
code :)

[titzer, 2016-05-09 08:49:54]

On 2016/05/09 08:47:40, Clemens Hammacher wrote:
> lgtm
> 
>
https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler.cc
> File src/compiler/wasm-compiler.cc (right):
> 
>
https://codereview.chromium.org/1962553002/diff/1/src/compiler/wasm-compiler....
> src/compiler/wasm-compiler.cc:2587: CHECK(effective_size <= kMaxUInt32);
> On 2016/05/09 08:38:17, titzer wrote:
> > On 2016/05/09 08:32:31, Clemens Hammacher wrote:
> > > Wouldn't a DCHECK suffice here?
> > 
> > I really, really, don't want to wrap around in production, so being extra
> > careful here.
> 
> It really should not be possible here, since (offset+memsize <= size) here,
and
> offset+memsize cannot overflow either.
> But I see the point.
> And maybe the compiler is even smart enough to remove this check in production
> code :)

Thanks for the detailed look.

[titzer, 2016-05-09 08:49:59]

The CQ bit was checked by titzer@chromium.org

[commit-bot: I haz the power, 2016-05-09 08:50:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1962553002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1962553002/1

[commit-bot: I haz the power, 2016-05-09 08:52:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-09 08:53:58]

Description was changed from

==========
[wasm] Fold bounds checks during graph building.

R=ahaas@chromium.org,clemensh@chromium.org
BUG=
==========

to

==========
[wasm] Fold bounds checks during graph building.

R=ahaas@chromium.org,clemensh@chromium.org
BUG=

Committed: https://crrev.com/aee1824adbcadb97bd7295e87e1e6765997bd8f9
Cr-Commit-Position: refs/heads/master@{#36100}
==========

[commit-bot: I haz the power, 2016-05-09 08:53:58]

Patchset 1 (id:??) landed as
https://crrev.com/aee1824adbcadb97bd7295e87e1e6765997bd8f9
Cr-Commit-Position: refs/heads/master@{#36100}