[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

Also factor out test cases from test-run-machops.cc into test-run-load-store.cc

BUG=chromium:599717
LOG=Y

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

[titzer, 2016-04-19 10:34:43]

titzer@chromium.org changed reviewers:
+ ahaas@chromium.org, bmeurer@chromium.org

[titzer, 2016-04-19 10:41:31]

Description was changed from

==========
[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

Also factor out test cases from test-run-machops.cc into test-run-load-store.cc

BUG=
==========

to

==========
[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

Also factor out test cases from test-run-machops.cc into test-run-load-store.cc

BUG=chromium:599717
LOG=Y
==========

[ahaas, 2016-04-19 17:57:28]

lgtm

https://codereview.chromium.org/1858323003/diff/60001/test/cctest/compiler/te...
File test/cctest/compiler/test-run-load-store.cc (right):

https://codereview.chromium.org/1858323003/diff/60001/test/cctest/compiler/te...
test/cctest/compiler/test-run-load-store.cc:410: const int32_t kNumElems = 27;
Is there a reason for using 27 elements? If not, then I think it would be good
to use something just above a power of two, e.g. 33, so that an additional
representation length is used but no time is wasted.

https://codereview.chromium.org/1858323003/diff/60001/test/cctest/compiler/te...
test/cctest/compiler/test-run-load-store.cc:457: const int32_t kNumElems = 29;
same here.

https://codereview.chromium.org/1858323003/diff/60001/test/cctest/compiler/te...
test/cctest/compiler/test-run-load-store.cc:684: void
TestRunOobCheckedLoad_pseudo(uint64_t x, bool length_is_immediate) {
How is this test different to the test above, except for the parameter type
which is casted away immediately?

https://codereview.chromium.org/1858323003/diff/60001/test/cctest/compiler/te...
test/cctest/compiler/test-run-load-store.cc:689: MachineOperatorBuilder
machine(m.zone());
you could just use m.machine(). Same for the tests above.

https://codereview.chromium.org/1858323003/diff/60001/test/cctest/compiler/te...
test/cctest/compiler/test-run-load-store.cc:775: void
TestRunOobCheckedLoadT_pseudo(uint64_t x, bool length_is_immediate) {
Why do you pass in an uint64_t if the only thing you do with it is casting it to
uint32_t?

[Benedikt Meurer, 2016-04-20 04:06:28]

LGTM with nit.

https://codereview.chromium.org/1858323003/diff/60001/src/compiler/code-gener...
File src/compiler/code-generator-impl.h (right):

https://codereview.chromium.org/1858323003/diff/60001/src/compiler/code-gener...
src/compiler/code-generator-impl.h:47: return
bit_cast<uint32_t>(ToConstant(instr_->InputAt(index)).ToInt32());
Nit: return bit_cast<uint32_t>(InputInt32(index));

[titzer, 2016-04-20 09:02:02]

https://codereview.chromium.org/1858323003/diff/60001/src/compiler/code-gener...
File src/compiler/code-generator-impl.h (right):

https://codereview.chromium.org/1858323003/diff/60001/src/compiler/code-gener...
src/compiler/code-generator-impl.h:47: return
bit_cast<uint32_t>(ToConstant(instr_->InputAt(index)).ToInt32());
On 2016/04/20 04:06:28, Benedikt Meurer wrote:
> Nit: return bit_cast<uint32_t>(InputInt32(index));

Done.

[titzer, 2016-04-20 09:32:17]

The CQ bit was checked by titzer@chromium.org

[titzer, 2016-04-20 09:32:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, ahaas@chromium.org
Link to the patchset: https://codereview.chromium.org/1858323003/#ps80001
(title: "git cl try")

[commit-bot: I haz the power, 2016-04-20 09:32:27]

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

[commit-bot: I haz the power, 2016-04-20 09:34:26]

Description was changed from

==========
[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

Also factor out test cases from test-run-machops.cc into test-run-load-store.cc

BUG=chromium:599717
LOG=Y
==========

to

==========
[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

Also factor out test cases from test-run-machops.cc into test-run-load-store.cc

BUG=chromium:599717
LOG=Y
==========

[commit-bot: I haz the power, 2016-04-20 09:34:27]

Committed patchset #5 (id:80001)

[nodir1, 2016-04-22 18:43:55]

Description was changed from

==========
[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

Also factor out test cases from test-run-machops.cc into test-run-load-store.cc

BUG=chromium:599717
LOG=Y
==========

to

==========
[turbofan] Length and index2 are unsigned in CheckedLoad/CheckedStore.

Also factor out test cases from test-run-machops.cc into test-run-load-store.cc

BUG=chromium:599717
LOG=Y

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

[nodir1, 2016-04-22 18:43:57]

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