Subzero: Elide checks of known valid accesses of locals

Subzero: Elide checks of known valid accesses of locals

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4374
R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=ac27c516cf0b490a7e06f66acff023cb6ecd3718

[tlively, 2016-07-26 01:00:09]

tlively@google.com changed reviewers:
+ kschimpf@google.com, mcgrathr@chromium.org, stichnot@chromium.org

[Jim Stichnoth, 2016-07-26 12:49:27]

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
File tests_lit/asan_tests/instrumentload.ll (right):

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
tests_lit/asan_tests/instrumentload.ll:23: define internal void @doLoads(i8*
%srcLocal8, i16* %srcLocal16,
This actually isn't valid PNaCl bitcode.

Pointer args should actually be passed as i32 args, and then converted inside
the function via inttoptr instructions.

A word of caution though.  Once you add explicit pointer cast instructions to
the .ll file, you can get slightly different input/output depending on how
Subzero is run.  If you run "pnacl-sz foo.ll" directly on the .ll file, Subzero
will see inttoptr/ptrtoint instructions as integer assignment instructions.  If
you run the %p2i pipeline in a lit test, the inttoptr/ptrtoint instructions
disappear completely and Subzero never sees them.  Just mentioning this in case
you get into a situation where everything looks good run manually but
inexplicably fails as a lit test.

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
File tests_lit/asan_tests/instrumentstore.ll (right):

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
tests_lit/asan_tests/instrumentstore.ll:16: define internal void @doStores(<4 x
i32> %vecSrc, i8* %destLocal8,
Same comment about int/pointer args.

[tlively, 2016-07-26 17:58:53]

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
File tests_lit/asan_tests/instrumentload.ll (right):

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
tests_lit/asan_tests/instrumentload.ll:23: define internal void @doLoads(i8*
%srcLocal8, i16* %srcLocal16,
On 2016/07/26 12:49:26, stichnot wrote:
> This actually isn't valid PNaCl bitcode.
> 
> Pointer args should actually be passed as i32 args, and then converted inside
> the function via inttoptr instructions.
> 
> A word of caution though.  Once you add explicit pointer cast instructions to
> the .ll file, you can get slightly different input/output depending on how
> Subzero is run.  If you run "pnacl-sz foo.ll" directly on the .ll file,
Subzero
> will see inttoptr/ptrtoint instructions as integer assignment instructions. 
If
> you run the %p2i pipeline in a lit test, the inttoptr/ptrtoint instructions
> disappear completely and Subzero never sees them.  Just mentioning this in
case
> you get into a situation where everything looks good run manually but
> inexplicably fails as a lit test.

Done. This is good to know.

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
File tests_lit/asan_tests/instrumentstore.ll (right):

https://codereview.chromium.org/2183643002/diff/20001/tests_lit/asan_tests/in...
tests_lit/asan_tests/instrumentstore.ll:16: define internal void @doStores(<4 x
i32> %vecSrc, i8* %destLocal8,
On 2016/07/26 12:49:26, stichnot wrote:
> Same comment about int/pointer args.

Done.

[Jim Stichnoth, 2016-07-26 18:29:09]

lgtm

[tlively, 2016-07-26 18:47:44]

Description was changed from

==========
Subzero: Elide checks of known valid accesses of locals

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4374
==========

to

==========
Subzero: Elide checks of known valid accesses of locals

BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4374
R=stichnot@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[tlively, 2016-07-26 18:47:45]

Committed patchset #3 (id:40001) manually as
ac27c516cf0b490a7e06f66acff023cb6ecd3718 (presubmit successful).