Improve opnd_create_base_disp_aarch64

Created by: yury-khrustalev

This API function for creating base-disp memory reference operands for AArch64

opnd_t
opnd_create_base_disp_aarch64(reg_id_t base_reg, reg_id_t index_reg,
                              dr_extend_type_t extend_type, bool scaled, int disp,
                              dr_opnd_flags_t flags, opnd_size_t size);

has confusing API especially when comparing it to Arm A64 specification.

The intent of this ticket is to discuss how this function can be improved. Currently, the problems are:

This function is essentially a single interface for two different operations (it violates separation of concerns principle).
It provides too many degrees of freedom and too many possible combinations than it actually should (and most of de jure "supported" pathways are not even checked within the function) (there should one and only one way to do something).
Parameter dr_extend_type_t extend_type implies all values from this enum may be valid, while only two of them actually make sense in this context (well, three if we agree to use UXTX for LSL).
Parameter dr_opnd_flags_t flags is set to the resulting opnd, but is in fact not used (at least not for AArch64), however some of the values of the enum look as if they may affect the operand somehow, so a user would be vainly contemplating which value to choose.
Parameter opnd_size_t size has confusing name and type. It is hard to guess that this is, in fact, a transfer size i.e. the size of the destination operand(s) in the instruction in which this (memref) operand is intended for.

The code review for addition of this function raised some of these concerns, but they had not been addressed.

The decision to store size in the operand instead of storing the shift amount was really bad. How can an operand carry a dependency on another operand?

I would like to suggest to replace this function with the following two functions:

/**
 * Returns a memory reference operand that refers to a base register X0 - X30 or
 * stack pointer XSP plus an extended and optionally shifted index register.
 *
 * Index register must be either Xm or Wm register. Together with the parameter
 * \p sign_extended it affects the type of extension used for the operand: SXTW
 * and UXTW when using signed and unsigned extension correspondingly with a Wm
 * register and LSL (UXTX) for Xm index register (behaviour for UXTX and SXTX is
 * identical in this context).
 *
 * The \p amount parameter determines optional shift of the extended value of
 * the index register and must be either 0 or log2(sz) where sz is the size of
 * the destination register in bytes (i.e. transfer size: 0 for 8-bit registers,
 * 1 for 16-bit registers, 2 for 32-bit registers, 3 for 64-bit registers, etc).
 *
 * \note AArch64-only.
 */
opnd_t
opnd_create_base_disp_reg(reg_id_t base_reg, reg_id_t index_reg, int amount, bool sign_extended);

/**
 * Returns a memory reference operand that refers to a base register X0 - X30 or
 * stack pointer XSP plus a constant signed displacement.
 *
 * The usable offsets depend on the type of instruction and the size of the
 * destination register (the transfer size).
 *
 * \note AArch64-only.
 */
opnd_t
opnd_create_base_disp_imm(reg_id_t base_reg, int offset);

Alternatively, a "merged" version of the two functions above may be used (which I don't really like), it would use offset for the displacement when index_reg = DR_REG_NULL and amount == 0:

opnd_t
opnd_create_base_disp_aarch64(reg_id_t base_reg, reg_id_t index_reg,
                              int amount, bool sign_extended, int offset);

But this is only to adhere to the "tradition" introduced by the existing function. I find it really a bad idea to create such facades. Imagine a god-like opnd_create_opnd function with a gazillion of parameters.