Subzero. ARM32. No more SP frobbing.

Subzero. ARM32. No more SP frobbing.

Pre-computes the max stack size outgoing arguments, and pre-allocates
it during prolog, deallocating during epilog.

With this CL, there are no more StackAdjustments needed for the ARM32,
which will simplify rematerializing alloca'd variables.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076
R=sehr@chromium.org

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

[John, 2015-11-20 16:18:59]

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

[sehr, 2015-11-20 20:07:35]

LGTM with one refactoring and one nit.

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

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:243: while (!Context.atEnd()) {
Again, factor to call the virtual method here.

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.h (right):

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:816: void findMaxStackOutArgsSize();
FWIW, I like the idea of making the method that computes a particular call
instruction's size a virtual method.  Something like:
 // Compute the space needed to pass this call's arguments on the stack.
 virtual uint32_t getCallArgumentAreaSize(const InstCall *Call) const = 0;

We can later avoid a separate pass by calling that method from processAllocas.

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:837: /// a new base register
ip=OrigBaseReg+Offsetis created, and the method
nit: Offset is

[John, 2015-11-20 22:17:18]

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

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:243: while (!Context.atEnd()) {
On 2015/11/20 20:07:35, sehr (please use this account) wrote:
> Again, factor to call the virtual method here.

Done.

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.h (right):

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:816: void findMaxStackOutArgsSize();
On 2015/11/20 20:07:35, sehr (please use this account) wrote:
> FWIW, I like the idea of making the method that computes a particular call
> instruction's size a virtual method.  Something like:
>  // Compute the space needed to pass this call's arguments on the stack.
>  virtual uint32_t getCallArgumentAreaSize(const InstCall *Call) const = 0;
> 
> We can later avoid a separate pass by calling that method from processAllocas.

I added the abstract virtual method, and I am now invoking it from the
findMaxOutArgsByteSize() method.

https://codereview.chromium.org/1467473003/diff/20001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:837: /// a new base register
ip=OrigBaseReg+Offsetis created, and the method
On 2015/11/20 20:07:35, sehr (please use this account) wrote:
> nit: Offset is

Done.

[John, 2015-11-20 22:17:27]

Committed patchset #4 (id:60001) manually as
f4198548d27ec3f4d169361e3a5ed95b893d54da (presubmit successful).

[Jim Stichnoth, 2015-11-22 04:09:43]

lgtm

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

https://codereview.chromium.org/1467473003/diff/60001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:268: // MinNeededOutArgsBytes should be updated
if the Target ever creates an
s/an/a/
at least for the way I pronounce "high"

https://codereview.chromium.org/1467473003/diff/60001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.h (right):

https://codereview.chromium.org/1467473003/diff/60001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:814: // bytes. This information is later used
during addProlog() do pre-allocate
s/do/to/

[John, 2015-11-23 18:54:10]

Done in another CL.

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

https://codereview.chromium.org/1467473003/diff/60001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:268: // MinNeededOutArgsBytes should be updated
if the Target ever creates an
On 2015/11/22 04:09:43, stichnot wrote:
> s/an/a/
> at least for the way I pronounce "high"

Done.

https://codereview.chromium.org/1467473003/diff/60001/src/IceTargetLoweringAR...
File src/IceTargetLoweringARM32.h (right):

https://codereview.chromium.org/1467473003/diff/60001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:814: // bytes. This information is later used
during addProlog() do pre-allocate
On 2015/11/22 04:09:43, stichnot wrote:
> s/do/to/

Done.