i#1569 AArch64: Make AArch64 build.

Commit log for first patchset:
---------------
i#1569 AArch64: Make AArch64 build.

It builds, but binaries are non-functional as many things are
not yet implemented.
---------------

[bruening, 2016-03-05 21:14:43]

Since we're about to put out the final 6.1 release, it seems best to put in a
code freeze for a couple of days until it's out to avoid the risk of all these
changes de-stabilizing something (no, we do not use repo branches in our
workflow).

[bruening, 2016-03-09 02:08:04]

On 2016/03/05 21:14:43, bruening wrote:
> Since we're about to put out the final 6.1 release, it seems best to put in a
> code freeze for a couple of days until it's out to avoid the risk of all these
> changes de-stabilizing something (no, we do not use repo branches in our
> workflow).

6.1 is out so the freeze is over.

[zhaoqin, 2016-03-09 03:46:21]

ok, I will review it ASAP

On Tue, Mar 8, 2016 at 9:08 PM, <bruening@google.com> wrote:

> On 2016/03/05 21:14:43, bruening wrote:
>
>> Since we're about to put out the final 6.1 release, it seems best to
>>
> put in a
>
>> code freeze for a couple of days until it's out to avoid the risk of
>>
> all these
>
>> changes de-stabilizing something (no, we do not use repo branches in
>>
> our
>
>> workflow).
>>
>
> 6.1 is out so the freeze is over.
>
> https://codereview.appspot.com/293720043/
>

[zhaoqin, 2016-03-10 16:20:13]

LGTM with some nits

https://codereview.appspot.com/293720043/diff/1/core/arch/arch.h
File core/arch/arch.h (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/arch.h#newcode148
core/arch/arch.h:148: #endif /* X86/ARM */
do we want to update here to match the macro?
I guess the comment is still kind of true, and we may need update many places.
So it is your call.

https://codereview.appspot.com/293720043/diff/1/core/arch/arch_exports.h
File core/arch/arch_exports.h (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/arch_exports.h#newc...
core/arch/arch_exports.h:143: #elif defined (ARM) || defined(AARCH64)
Currently we only support X86 and ARM/AArch64, sometime I was wondering if we
need this "elif defined", but I guess it is really for future possible porting
to other ISA.

https://codereview.appspot.com/293720043/diff/1/core/arch/arch_exports.h#newc...
core/arch/arch_exports.h:490: 
I think we could share some code with ARM, but I am ok to have separate one here
now.

https://codereview.appspot.com/293720043/diff/1/core/arch/arch_exports.h#newc...
core/arch/arch_exports.h:530: #  ifdef X64
should we move this to AARCH64?

https://codereview.appspot.com/293720043/diff/1/core/arch/arch_exports.h#newc...
core/arch/arch_exports.h:1614: SVC_LENGTH = 4,
can we merge this into ARM, seems possible.

https://codereview.appspot.com/293720043/diff/1/core/arch/arch_exports.h#newc...
core/arch/arch_exports.h:1990: reg_t regs[22/*X19-X30,unused,SP,D8-D15*/];
so X0-X18 are scratch regs?
Can we add more comment about why these registers only, or if there is comment
in aarch64.asm, add xref here.

https://codereview.appspot.com/293720043/diff/1/core/arch/asm_defines.asm
File core/arch/asm_defines.asm (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/asm_defines.asm#new...
core/arch/asm_defines.asm:270: # define REG_R12 x12
we only have reg up to 12?

https://codereview.appspot.com/293720043/diff/1/core/arch/instr_inline.h
File core/arch/instr_inline.h (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/instr_inline.h#newc...
core/arch/instr_inline.h:163: #   define OPND_IS_REL_ADDR(op) ((op).kind ==
REL_ADDR_kind)
It looks like we have pc relative address in AArch64
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0801a/BABGHBJC...

I understand PC is implicit opnd in AArch64, but DR usually list all implicit
and explicit opnd, so it probably should be the same as ARM.

https://codereview.appspot.com/293720043/diff/1/core/arch/instr_shared.c
File core/arch/instr_shared.c (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/instr_shared.c#newc...
core/arch/instr_shared.c:1935: #ifdef X86
the ymmh is X86 only, shouldn't we make this whole routine x86 only?

https://codereview.appspot.com/293720043/diff/1/core/arch/opnd.h
File core/arch/opnd.h (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/opnd.h#newcode288
core/arch/opnd.h:288: DR_REG_NZCV, DR_REG_FPCR, DR_REG_FPSR,
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0801b/BABBGCAC...

ok, so we do not have CPSR in AArch64

https://codereview.appspot.com/293720043/diff/1/core/arch/opnd_shared.c
File core/arch/opnd_shared.c (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/opnd_shared.c#newco...
core/arch/opnd_shared.c:2068: # ifndef X64
would using AARCH64 be better here?
The reason we used X64 before was to make ARM as the general MACRO for AArch32
and AArch64, if it is not the case, I think I would prefer AARCH64 when possible
to avoid confusing with x86's X64.

https://codereview.appspot.com/293720043/diff/1/core/lib/instrument.c
File core/lib/instrument.c (right):

https://codereview.appspot.com/293720043/diff/1/core/lib/instrument.c#newcode...
core/lib/instrument.c:5652: (void)dcontext;
might I know what this for?

https://codereview.appspot.com/293720043/diff/1/core/unix/include/syscall_lin...
File core/unix/include/syscall_linux_arm.h (right):

https://codereview.appspot.com/293720043/diff/1/core/unix/include/syscall_lin...
core/unix/include/syscall_linux_arm.h:21: #if defined(X64) && 0 /* FIXME i#1569:
arm64 has different numbers */
why not use AARCH64?

https://codereview.appspot.com/293720043/diff/1/core/unix/signal.c
File core/unix/signal.c (right):

https://codereview.appspot.com/293720043/diff/1/core/unix/signal.c#newcode2034
core/unix/signal.c:2034: # ifndef X64
use AARCH64

[Edmund.Grimley.Evans, 2016-03-10 17:47:12]

> File core/arch/instr_inline.h (right):
> 
>
https://codereview.appspot.com/293720043/diff/1/core/arch/instr_inline.h#newc...
> core/arch/instr_inline.h:163: #   define OPND_IS_REL_ADDR(op) ((op).kind ==
> REL_ADDR_kind)
> It looks like we have pc relative address in AArch64
>
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0801a/BABGHBJC...
> 
> I understand PC is implicit opnd in AArch64, but DR usually list all implicit
> and explicit opnd, so it probably should be the same as ARM.

The plan is to be like x86_64 in this respect and have no DR_REG_PC on AArch64.

> https://codereview.appspot.com/293720043/diff/1/core/lib/instrument.c
> File core/lib/instrument.c (right):
> 
>
https://codereview.appspot.com/293720043/diff/1/core/lib/instrument.c#newcode...
> core/lib/instrument.c:5652: (void)dcontext;
> might I know what this for?

It's to prevent a warning/error about the unused variable, of course. The
variable will be used when the code is implemented.

> https://codereview.appspot.com/293720043/diff/1/core/unix/signal.c
> File core/unix/signal.c (right):
> 
> https://codereview.appspot.com/293720043/diff/1/core/unix/signal.c#newcode2034
> core/unix/signal.c:2034: # ifndef X64
> use AARCH64

I agree that #ifndef AARCH64 is clearer than #ifndef X64, but I think I'll
change to #ifdef ARM, which is even clearer.

Thanks for the review!

[Edmund.Grimley.Evans, 2016-03-11 10:30:04]

Committed as
https://github.com/DynamoRIO/dynamorio/commit/c2ecca24db224f9d5fe2a5193dacfa6...

Final commit log: 
---------------
i#1569 AArch64: Make AArch64 build.

It builds, but binaries are non-functional as many things are
not yet implemented.

Review-URL: https://codereview.appspot.com/293720043
---------------

[sevenfold1999AC, 2016-10-20 15:39:24]

Send all my messages to my Gmail sevenfold1999ac@gmail.com asap

https://codereview.appspot.com/293720043/diff/1/core/arch/opnd_shared.c
File core/arch/opnd_shared.c (right):

https://codereview.appspot.com/293720043/diff/1/core/arch/opnd_shared.c#newco...
core/arch/opnd_shared.c:2068: # ifndef X64
On 2016/03/10 16:20:13, zhaoqin wrote:
> would using AARCH64 be better here?
> The reason we used X64 before was to make ARM as the general MACRO for AArch32
> and AArch64, if it is not the case, I think I would prefer AARCH64 when
possible
> to avoid confusing with x86's X64.

Done.

https://codereview.appspot.com/293720043/diff/1/core/arch/opnd_shared.c#newco...
core/arch/opnd_shared.c:2068: # ifndef X64
On 2016/03/10 16:20:13, zhaoqin wrote:
> would using AARCH64 be better here?
> The reason we used X64 before was to make ARM as the general MACRO for AArch32
> and AArch64, if it is not the case, I think I would prefer AARCH64 when
possible
> to avoid confusing with x86's X64.

Done.