[Subzero][MIPS32] Implements lowering of alloca instruction

[Subzero][MIPS32] Implements lowering of alloca instruction

BUG=none
R=stichnot@chromium.org

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

[sagar.thakur, 2016-06-15 12:24:10]

sagar.thakur@imgtec.com changed reviewers:
+ eholk@chromium.org, jpp@chromium.org, kschimpf@google.com,
stichnot@chromium.org

[Jim Stichnoth, 2016-06-15 13:41:19]

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:166: void TargetMIPS32::unsetIfNonLeafFunc() {
I think that the ARM target folds this computation into the instruction lowering
pass, instead of making a separate pass.  My experience has been that making a
pass over each instruction, like this one, costs about 1% of the total
translation time.  That's not much, but they can add up, so you might consider
doing similar to ARM after the call lowering code stabilizes.

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:172: break;
Can you just early-return here, and avoid the break statements?

[Jim Stichnoth, 2016-06-15 13:43:19]

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:166: void TargetMIPS32::unsetIfNonLeafFunc() {
On 2016/06/15 13:41:19, stichnot wrote:
> I think that the ARM target folds this computation into the instruction
lowering
> pass, instead of making a separate pass.  My experience has been that making a
> pass over each instruction, like this one, costs about 1% of the total
> translation time.  That's not much, but they can add up, so you might consider
> doing similar to ARM after the call lowering code stabilizes.

Actually, it just occurred to me that statistically, this pass is much cheaper
than a full pass over instructions because of the early-exit.

[sagar.thakur, 2016-06-16 08:05:54]

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:166: void TargetMIPS32::unsetIfNonLeafFunc() {
On 2016/06/15 13:43:19, stichnot wrote:
> On 2016/06/15 13:41:19, stichnot wrote:
> > I think that the ARM target folds this computation into the instruction
> lowering
> > pass, instead of making a separate pass.  My experience has been that making
a
> > pass over each instruction, like this one, costs about 1% of the total
> > translation time.  That's not much, but they can add up, so you might
consider
> > doing similar to ARM after the call lowering code stabilizes.
> 
> Actually, it just occurred to me that statistically, this pass is much cheaper
> than a full pass over instructions because of the early-exit.

This pass is needed because findMaxStackOutArgsSize needs to know whether a
function is a leaf function or a non-leaf function to compute the
MaxOutArgsSizeBytes value. In case of non-leaf function, the mips o32 abi
specifies that a caller must allocate at least 16 bytes of memory as argument
build area which will be used by callee even if arguments passed occupy less
than 16 bytes. Whereas if a function is a leaf function then there's no need to
allocate this memory.

https://codereview.chromium.org/2067183002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:172: break;
On 2016/06/15 13:41:19, stichnot wrote:
> Can you just early-return here, and avoid the break statements?

Done.

[Jim Stichnoth, 2016-06-16 22:30:05]

lgtm

[Jim Stichnoth, 2016-06-16 22:30:30]

Description was changed from

==========
[Subzero][MIPS32] Implements lowering of alloca instruction

BUG=none
==========

to

==========
[Subzero][MIPS32] Implements lowering of alloca instruction

BUG=none
R=stichnot@chromium.org

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

[Jim Stichnoth, 2016-06-16 22:30:34]

Committed patchset #2 (id:20001) manually as
1afb4836b204934e967d930616a4f97db75e0353 (presubmit successful).