Handle stack spills in ARM integrated assembler.

Handle stack spills in ARM integrated assembler.

Add code to handle spilling stack variables. That is, add code to
handle loading and storing to stack addresses.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4334
R=jpp@chromium.org, stichnot@chromium.org

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

[Karl, 2015-10-15 19:54:31]

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

[Jim Stichnoth, 2015-10-15 20:41:29]

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
File src/DartARM32/assembler_arm.cc (right):

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
src/DartARM32/assembler_arm.cc:225: #if 0
#endif ?

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
File src/DartARM32/assembler_arm.h (right):

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
src/DartARM32/assembler_arm.h:447: #if 0
Missing #endif ?

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
File src/IceAssemblerARM32.cpp (right):

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:67: template <typename T> static inline bool
isUint(int N, T Value) {
Can you just use the (inappropriately named) IsUint() and IsAbsoluteUint() in
IceUtils.h?

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:102: // Returns the bits int the corresponding masked
value
"int" ?  Also, end sentence with period.

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:163: // Note: Loop is used so that we can short ciruit
using break;
Would it make sense here to just "return CantDecode" instead of "break"?

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:363: bool IsByte = Ty == IceType_i8;
IceType_i1 should also qualify as IsByte, since str() is used for stack stores. 
(assuming this is how i1 variables are lowered)

[John, 2015-10-15 20:59:25]

lgtm

minor comments (adnd in agreement with Jim). lgtm otherwise.

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
File src/IceAssemblerARM32.cpp (right):

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:363: bool IsByte = Ty == IceType_i8;
On 2015/10/15 20:41:29, stichnot wrote:
> IceType_i1 should also qualify as IsByte, since str() is used for stack
stores. 
> (assuming this is how i1 variables are lowered)

const bool IsByte = ...

also, please assert that Ty != IceType_i1 here. I would not like to load/store
i1

[Karl, 2015-10-15 22:03:33]

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
File src/DartARM32/assembler_arm.cc (right):

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
src/DartARM32/assembler_arm.cc:225: #if 0
On 2015/10/15 20:41:29, stichnot wrote:
> #endif ?

Done.

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
File src/DartARM32/assembler_arm.h (right):

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/DartARM32/as...
src/DartARM32/assembler_arm.h:447: #if 0
On 2015/10/15 20:41:29, stichnot wrote:
> Missing #endif ?

Done.

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
File src/IceAssemblerARM32.cpp (right):

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:67: template <typename T> static inline bool
isUint(int N, T Value) {
On 2015/10/15 20:41:29, stichnot wrote:
> Can you just use the (inappropriately named) IsUint() and IsAbsoluteUint() in
> IceUtils.h?

Done.

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:102: // Returns the bits int the corresponding masked
value
On 2015/10/15 20:41:29, stichnot wrote:
> "int" ?  Also, end sentence with period.

Done.

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:163: // Note: Loop is used so that we can short ciruit
using break;
On 2015/10/15 20:41:29, stichnot wrote:
> Would it make sense here to just "return CantDecode" instead of "break"?

Good idea. Removing loop as well.

https://chromiumcodereview.appspot.com/1402403002/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:363: bool IsByte = Ty == IceType_i8;
On 2015/10/15 20:59:25, John wrote:
> On 2015/10/15 20:41:29, stichnot wrote:
> > IceType_i1 should also qualify as IsByte, since str() is used for stack
> stores. 
> > (assuming this is how i1 variables are lowered)
> 
> const bool IsByte = ...
> 
> also, please assert that Ty != IceType_i1 here. I would not like to load/store
> i1

Using typeWidthInBytes to resolve if is byte. Note that currently, we only allow
i32 and i8 (see test above). I have intentionally left out the other cases until
I can see an example.

[Jim Stichnoth, 2015-10-15 23:39:45]

lgtm

[Karl, 2015-10-16 17:31:35]

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