Subzero. ARM32. Fix bugs uncovered by scons tests.

Subzero. ARM32. Fix bugs uncovered by scons tests.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4076
R=eholk@chromium.org, kschimpf@google.com, stichnot@chromium.org

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

[John, 2016-03-07 21:47:01]

jpp@chromium.org changed reviewers:
+ eholk@chromium.org, kschimpf@google.com, sehr@chromium.org,
stichnot@chromium.org

[Karl, 2016-03-07 21:53:05]

lgtm

[Eric Holk, 2016-03-07 22:58:53]

lgtm

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:4611: Retry =
Context.insert<InstARM32Label>(this);
Why did you split this into two lines instead of leaving it as before?

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:4921: [this, Expected, New, Instr, DestTy,
LoadedValuePtr](Variable *Tmp) {
How do you feel about during a by-reference capture of LoadedValue instead of
adding LoadedValuePtr?

[Jim Stichnoth, 2016-03-07 23:25:49]

lgtm

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:4931: // Context.insert<InstFakeUse>(Tmp);
Delete these comments?

[John, 2016-03-08 18:14:32]

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.cpp (right):

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:4611: Retry =
Context.insert<InstARM32Label>(this);
On 2016/03/07 22:58:53, Eric Holk wrote:
> Why did you split this into two lines instead of leaving it as before?

Leftover from one of the changes.

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:4921: [this, Expected, New, Instr, DestTy,
LoadedValuePtr](Variable *Tmp) {
On 2016/03/07 22:58:53, Eric Holk wrote:
> How do you feel about during a by-reference capture of LoadedValue instead of
> adding Loaded

I don't like this idiom, but I dislike non const references even more. What I
really wanted here was a capture by pointer... :P

but done.

https://codereview.chromium.org/1768823002/diff/40001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:4931: // Context.insert<InstFakeUse>(Tmp);
On 2016/03/07 23:25:48, stichnot wrote:
> Delete these comments?

Done.

[John, 2016-03-08 19:01:11]

Description was changed from

==========
Subzero. ARM32. Fix bugs uncovered by scons tests.

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

to

==========
Subzero. ARM32. Fix bugs uncovered by scons tests.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4076
R=eholk@chromium.org, kschimpf@google.com, stichnot@chromium.org

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

[John, 2016-03-08 19:01:13]

Committed patchset #5 (id:80001) manually as
324334e5315db2e67f16a4dffe91e1b1faf8dc8e (presubmit successful).