Handle MOV (immediate) and MOVT to load ARM global addresses.

Handle MOV (immediate) and MOVT to load ARM global addresses.

Adds a new type of fixup to handle the relocatable fixups needed
for movw and movt on a global addresses. Also adds movw and movt
methods to the ARM assembler.

Also makes ARM register names visible (without a target lowering
object), so that the ARM integrated assembler can generate the appropriate assembly.
Note that the integrated assembler needs to generate the
corresponding movw/movt, and follows the instruction with the bytes
that appear in the corresponding assembler buffer. This allows the
ability to test if we have generated the correct values, and will be
set up properly for ELF emission.

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

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

[Karl, 2015-10-28 16:59:47]

Description was changed from

==========
Handle MOV (immediate) and MOVT to load ARM global addresses.

Adds a new type of fixup to handle the relocatable fixups needed
for movw and movt on a global address. Also adds movw and movt
methods to the ARM assembler.

Also makes ARM register names visible (without a target lowering
object), so that the ARM integrated assembler can generate assemblable
code.  Note that the integrated assembler needs to generate the
corresponding movw/movt, and follows the instruction with the bytes
that appear in the corresponding assembler buffer. This allows the
ability to test if we have generated the correct values, and will be
set up properly for ELF emission.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4334
==========

to

==========
Handle MOV (immediate) and MOVT to load ARM global addresses.

Adds a new type of fixup to handle the relocatable fixups needed
for movw and movt on a global address. Also adds movw and movt
methods to the ARM assembler.

Also makes ARM register names visible (without a target lowering
object), so that the ARM integrated assembler can generate assemblable
code.  Note that the integrated assembler needs to generate the
corresponding movw/movt, and follows the instruction with the bytes
that appear in the corresponding assembler buffer. This allows the
ability to test if we have generated the correct values, and will be
set up properly for ELF emission.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4334
==========

[Karl, 2015-10-28 16:59:47]

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

[Karl, 2015-10-28 17:00:12]

Description was changed from

==========
Handle MOV (immediate) and MOVT to load ARM global addresses.

Adds a new type of fixup to handle the relocatable fixups needed
for movw and movt on a global address. Also adds movw and movt
methods to the ARM assembler.

Also makes ARM register names visible (without a target lowering
object), so that the ARM integrated assembler can generate assemblable
code.  Note that the integrated assembler needs to generate the
corresponding movw/movt, and follows the instruction with the bytes
that appear in the corresponding assembler buffer. This allows the
ability to test if we have generated the correct values, and will be
set up properly for ELF emission.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4334
==========

to

==========
Handle MOV (immediate) and MOVT to load ARM global addresses.

Adds a new type of fixup to handle the relocatable fixups needed
for movw and movt on a global addresses. Also adds movw and movt
methods to the ARM assembler.

Also makes ARM register names visible (without a target lowering
object), so that the ARM integrated assembler can generate assemblable
code.  Note that the integrated assembler needs to generate the
corresponding movw/movt, and follows the instruction with the bytes
that appear in the corresponding assembler buffer. This allows the
ability to test if we have generated the correct values, and will be
set up properly for ELF emission.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4334
==========

[Karl, 2015-10-28 17:01:07]

Description was changed from

==========
Handle MOV (immediate) and MOVT to load ARM global addresses.

Adds a new type of fixup to handle the relocatable fixups needed
for movw and movt on a global addresses. Also adds movw and movt
methods to the ARM assembler.

Also makes ARM register names visible (without a target lowering
object), so that the ARM integrated assembler can generate assemblable
code.  Note that the integrated assembler needs to generate the
corresponding movw/movt, and follows the instruction with the bytes
that appear in the corresponding assembler buffer. This allows the
ability to test if we have generated the correct values, and will be
set up properly for ELF emission.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4334
==========

to

==========
Handle MOV (immediate) and MOVT to load ARM global addresses.

Adds a new type of fixup to handle the relocatable fixups needed
for movw and movt on a global addresses. Also adds movw and movt
methods to the ARM assembler.

Also makes ARM register names visible (without a target lowering
object), so that the ARM integrated assembler can generate the appropriate
assembly.
Note that the integrated assembler needs to generate the
corresponding movw/movt, and follows the instruction with the bytes
that appear in the corresponding assembler buffer. This allows the
ability to test if we have generated the correct values, and will be
set up properly for ELF emission.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4334
==========

[Jim Stichnoth, 2015-10-28 22:28:55]

https://codereview.chromium.org/1424863005/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1424863005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:235: if (kind() == llvm::ELF::R_ARM_MOVW_ABS_NC)
What do you think of a long statement like
  Str << "\t" 
      << (kind() == llvm::ELF::R_ARM_MOVW_ABS_NC ? "movw" : "movt")
      << "\t" << RegNames[..] << ...

https://codereview.chromium.org/1424863005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:246: for (int HexDigit = 2 * FixupSize; HexDigit > 0;
--HexDigit) {
Can you use llvm::format_hex_no_prefix() ?  This is used in IceOperand.h.

https://codereview.chromium.org/1424863005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:572: (void)Cond;
Remove this?

https://codereview.chromium.org/1424863005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:586: // Use 0 for the lower 16 bits of the
relocatable, and add a
reflow comment to 80-col

https://codereview.chromium.org/1424863005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:596: setNeedsTextFixup();
This call is a little puzzling because I think it serves two purposes.  First,
if you get all the way through without any of the conditions firing, it means
the IAS failed and we should fall back to a text fixup.  Second, if we emit a
movw from a ConstantRelocatable, we still need a text fixup (because we don't
know how to do it any other way in textual asm).

If my understanding is right, I think it would be better to have two separate
calls to setNeedsTextFixup(), each with a comment describing why.  Eventually of
course the latter one will be removed because all the IAS will be implemented.

https://codereview.chromium.org/1424863005/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:601: (void)Cond;
remove?

[Karl, 2015-10-29 16:57:53]

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

https://chromiumcodereview.appspot.com/1424863005/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:235: if (kind() == llvm::ELF::R_ARM_MOVW_ABS_NC)
On 2015/10/28 22:28:55, stichnot wrote:
> What do you think of a long statement like
>   Str << "\t" 
>       << (kind() == llvm::ELF::R_ARM_MOVW_ABS_NC ? "movw" : "movt")
>       << "\t" << RegNames[..] << ...

Done.

https://chromiumcodereview.appspot.com/1424863005/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:246: for (int HexDigit = 2 * FixupSize; HexDigit > 0;
--HexDigit) {
On 2015/10/28 22:28:55, stichnot wrote:
> Can you use llvm::format_hex_no_prefix() ?  This is used in IceOperand.h.

Done.

https://chromiumcodereview.appspot.com/1424863005/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:572: (void)Cond;
On 2015/10/28 22:28:55, stichnot wrote:
> Remove this?

Done.

https://chromiumcodereview.appspot.com/1424863005/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:586: // Use 0 for the lower 16 bits of the
relocatable, and add a
On 2015/10/28 22:28:55, stichnot wrote:
> reflow comment to 80-col

Done.

https://chromiumcodereview.appspot.com/1424863005/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:596: setNeedsTextFixup();
On 2015/10/28 22:28:55, stichnot wrote:
> This call is a little puzzling because I think it serves two purposes.  First,
> if you get all the way through without any of the conditions firing, it means
> the IAS failed and we should fall back to a text fixup.  Second, if we emit a
> movw from a ConstantRelocatable, we still need a text fixup (because we don't
> know how to do it any other way in textual asm).
> 
> If my understanding is right, I think it would be better to have two separate
> calls to setNeedsTextFixup(), each with a comment describing why.  Eventually
of
> course the latter one will be removed because all the IAS will be implemented.

Hmm, this is a mistake. It shouldn't even be there. We generated the fixup
above, and it generates the corresponding text form. Removing.
Adding fixup to InstARM32Mov{w,t}::emitIAS to generate fixup if needed.

https://chromiumcodereview.appspot.com/1424863005/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:601: (void)Cond;
On 2015/10/28 22:28:55, stichnot wrote:
> remove?

Done.

https://chromiumcodereview.appspot.com/1424863005/diff/20001/src/IceAssembler...
src/IceAssemblerARM32.cpp:625: setNeedsTextFixup();
Removed this also.

[Karl, 2015-10-29 18:35:06]

Fixed implementations of movw and movt in assembler.

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

https://chromiumcodereview.appspot.com/1424863005/diff/80001/src/IceAssembler...
src/IceAssemblerARM32.cpp:565: if (Src == nullptr)
Added fallback if not relocatable constant.

https://chromiumcodereview.appspot.com/1424863005/diff/80001/src/IceAssembler...
src/IceAssemblerARM32.cpp:593: if (Src == nullptr)
Added fallback if not relocatable constant.

[Jim Stichnoth, 2015-10-29 20:16:34]

lgtm

[Karl, 2015-10-29 21:04:17]

Committed patchset #5 (id:80001) manually as
7cb2db327e386372003cdb45e2c3acef774fb902 (presubmit successful).