Add a basic TargetARM32 skeleton which knows nothing.

Add a basic TargetARM32 skeleton which knows nothing.

Later commits will add more information, but this tests the
conditional compilation and build setup.

One way to do conditional compilation: determine this
early, at LLVM configure/CMake time. Configure will
fill in the template of SZTargets.def.in to get
a SZTargets.def file.

LLVM change:
https://codereview.chromium.org/1084753002/

NaCl change:
https://codereview.chromium.org/1082953002/

I suppose an alternative is to fill in the .def file via
-D flags in CXXFLAGS.

For conditional lit testing, pnacl-sz dumps the attributes
when given the --build-atts so we just build on top of that.
We do that instead of go the LLVM way of filling in a
lit.site.cfg.in -> lit.site.cfg at configure/CMake time.


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

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

[jvoung (off chromium), 2015-04-14 00:20:23]

jvoung@chromium.org changed reviewers:
+ dschuff@chromium.org, jfb@chromium.org, mtrofin@chromium.org,
stichnot@chromium.org

[jvoung (off chromium), 2015-04-14 00:30:07]

And here's the example of how https://codereview.chromium.org/1082953002/ might
use the .def file.

[Jim Stichnoth, 2015-04-17 19:16:02]

otherwise lgtm

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.def
File src/IceInstARM32.def (right):

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.def#ne...
src/IceInstARM32.def:18: // For the NaCl sandbox we also need to r9 for TLS, so
just reserve always.
Probably add a TODO that we'd like to mark r9 as isInt in native mode but
unavailable in sandboxing mode.  This will also be necessary for x86-64.

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:1: //===- subzero/src/IceInstARM32.h - Low-level ARM
instructions --*- C++ -*-===//
ARM32?  (though that messes with line length...)

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLowering.cpp
File src/IceTargetLowering.cpp (right):

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLowering.c...
src/IceTargetLowering.cpp:26: #include "IceTargetLoweringARM32.h"
alphabetize

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLowering.c...
src/IceTargetLowering.cpp:67: // These statements can be #ifdef'd to specialize
the code generator
hmm, surprised that the comment indent changed...

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

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:10: // This file implements the
TargetLoweringARM32 class, which
Reformat to 80-col lines?  (this is probably an artifact of the old name
"IceTargetLoweringX8632" plus emacs fill column = 70)

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:72: // TODO: share passes with X86?
Definitely.  This would presumably be done as part of
https://code.google.com/p/nativeclient/issues/detail?id=4094 .  Maybe note the
issue here

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

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:30: static TargetARM32 *create(Cfg *Func) { return
new TargetARM32(Func); }
Just noticed that TargetFoo::create() and TargetLowering::createLowering()
should probably be returning std::unique_ptr<TargetLowering>, similar to what
TargetDataLowering does.

Maybe add a TODO here for later?

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:57: SizeT makeNextLabelNumber() { return
NextLabelNumber++; }
Maybe this should be lifted into the base class?

(I don't see any other methods to lift, though hasFramePointer() is borderline.)

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:93: // TODO(jvoung): Refactor to base
TargetLowering?
sgtm

https://codereview.chromium.org/1075363002/diff/80001/src/assembler_arm32.h
File src/assembler_arm32.h (right):

https://codereview.chromium.org/1075363002/diff/80001/src/assembler_arm32.h#n...
src/assembler_arm32.h:1: //===- subzero/src/assembler_arm32.h - Assembler for
ARM32 ------*- C++ -*-===//
Blank line after the first line, to match other .h files?
(though I see assembler_ia32.h is this way)

https://codereview.chromium.org/1075363002/diff/80001/src/assembler_arm32.h#n...
src/assembler_arm32.h:21: #ifndef SUBZERO_SRC_ASSEMBLER_ARM32_H_
Trailing underscore or not?  assembler_ia32.h uses it, other files do not.

[jvoung (off chromium), 2015-04-21 17:05:30]

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.def
File src/IceInstARM32.def (right):

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.def#ne...
src/IceInstARM32.def:18: // For the NaCl sandbox we also need to r9 for TLS, so
just reserve always.
On 2015/04/17 19:16:01, stichnot wrote:
> Probably add a TODO that we'd like to mark r9 as isInt in native mode but
> unavailable in sandboxing mode.  This will also be necessary for x86-64.

Done.

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:1: //===- subzero/src/IceInstARM32.h - Low-level ARM
instructions --*- C++ -*-===//
On 2015/04/17 19:16:01, stichnot wrote:
> ARM32?  (though that messes with line length...)

Yeah that is why I left out the 32. I could say "ARM32 machine instructions".

Changed that for now (and the x86 one too...), but can change back if you prefer
Low-level.

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLowering.cpp
File src/IceTargetLowering.cpp (right):

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLowering.c...
src/IceTargetLowering.cpp:26: #include "IceTargetLoweringARM32.h"
On 2015/04/17 19:16:01, stichnot wrote:
> alphabetize

Done.

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLowering.c...
src/IceTargetLowering.cpp:67: // These statements can be #ifdef'd to specialize
the code generator
On 2015/04/17 19:16:01, stichnot wrote:
> hmm, surprised that the comment indent changed...

Hmm some clang-format artifact? I couldn't convince it to not indent so I just
removed the comment.

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

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:10: // This file implements the
TargetLoweringARM32 class, which
On 2015/04/17 19:16:01, stichnot wrote:
> Reformat to 80-col lines?  (this is probably an artifact of the old name
> "IceTargetLoweringX8632" plus emacs fill column = 70)

Done.

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.cpp:72: // TODO: share passes with X86?
On 2015/04/17 19:16:01, stichnot wrote:
> Definitely.  This would presumably be done as part of
> https://code.google.com/p/nativeclient/issues/detail?id=4094 .  Maybe note the
> issue here

Done.

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

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:30: static TargetARM32 *create(Cfg *Func) { return
new TargetARM32(Func); }
On 2015/04/17 19:16:02, stichnot wrote:
> Just noticed that TargetFoo::create() and TargetLowering::createLowering()
> should probably be returning std::unique_ptr<TargetLowering>, similar to what
> TargetDataLowering does.
> 
> Maybe add a TODO here for later?

Done.

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:57: SizeT makeNextLabelNumber() { return
NextLabelNumber++; }
On 2015/04/17 19:16:01, stichnot wrote:
> Maybe this should be lifted into the base class?
> 
> (I don't see any other methods to lift, though hasFramePointer() is
borderline.)

Done.

https://codereview.chromium.org/1075363002/diff/80001/src/IceTargetLoweringAR...
src/IceTargetLoweringARM32.h:93: // TODO(jvoung): Refactor to base
TargetLowering?
On 2015/04/17 19:16:02, stichnot wrote:
> sgtm

Went ahead and Done it.

https://codereview.chromium.org/1075363002/diff/80001/src/assembler_arm32.h
File src/assembler_arm32.h (right):

https://codereview.chromium.org/1075363002/diff/80001/src/assembler_arm32.h#n...
src/assembler_arm32.h:1: //===- subzero/src/assembler_arm32.h - Assembler for
ARM32 ------*- C++ -*-===//
On 2015/04/17 19:16:02, stichnot wrote:
> Blank line after the first line, to match other .h files?
> (though I see assembler_ia32.h is this way)

Done.

https://codereview.chromium.org/1075363002/diff/80001/src/assembler_arm32.h#n...
src/assembler_arm32.h:21: #ifndef SUBZERO_SRC_ASSEMBLER_ARM32_H_
On 2015/04/17 19:16:02, stichnot wrote:
> Trailing underscore or not?  assembler_ia32.h uses it, other files do not.

Done.

[Jim Stichnoth, 2015-04-21 17:10:36]

Thanks, still lgtm.

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1075363002/diff/80001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:1: //===- subzero/src/IceInstARM32.h - Low-level ARM
instructions --*- C++ -*-===//
On 2015/04/21 17:05:30, jvoung wrote:
> On 2015/04/17 19:16:01, stichnot wrote:
> > ARM32?  (though that messes with line length...)
> 
> Yeah that is why I left out the 32. I could say "ARM32 machine instructions".
> 
> Changed that for now (and the x86 one too...), but can change back if you
prefer
> Low-level.

That looks good, thanks!

[jvoung (off chromium), 2015-04-22 00:01:54]

Committed patchset #11 (id:200001) manually as
b36ad9b4021b409dfb925ce51196a1abb4dfcb62 (presubmit successful).