i#1569 AArch64: Make base_disp bitfields in opnd_t architecture-depen…

…dent.

We will add several bits for AArch64 but we do not want to increase the
size of opnd_t. Use ifdef rather than a union.

Also make opnd_create_base_disp_arm and opnd_{get,set}_index_shift ARM-only,
and opnd_set_disp_ex X86-only.

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

core/arch/opnd.h

@@ -823,20 +823,18 @@ struct _opnd_t {
             /* to get cl to not align to 4 bytes we can't use uint here
              * when we have reg_id_t elsewhere: it won't combine them
              * (gcc will). alternative is all uint and no reg_id_t.
-             */
-            /* We would use a union and struct to separate the scale from the 2
-             * shift fields as they are mutually exclusive, but that would
-             * require packing the struct or living with a larger size and perf
-             * hit on copying it.  We also have to use byte and not dr_shift_type_t
+             * We also have to use byte and not dr_shift_type_t
              * to get cl to not align.
              */
-            byte/*dr_shift_type_t*/ shift_type : 3; /* ARM-only */
-            byte shift_amount_minus_1 : 5; /* ARM-only, 1..31 so we store (val - 1) */
-            byte scale : SCALE_SPECIFIER_BITS; /* x86-only */
-            /* These 3 are all x86-only: */
+# if defined(ARM)
+            byte/*dr_shift_type_t*/ shift_type : 3;
+            byte shift_amount_minus_1 : 5; /* 1..31 so we store (val - 1) */
+# elif defined(X86)
+            byte scale : SCALE_SPECIFIER_BITS;
             byte/*bool*/ encode_zero_disp : 1;
             byte/*bool*/ force_full_disp : 1; /* don't use 8-bit even w/ 8-bit value */
             byte/*bool*/ disp_short_addr : 1; /* 16-bit (32 in x64) addr (disp-only) */
+# endif
         } base_disp;            /* BASE_DISP_kind */
         void *addr;             /* REL_ADDR_kind and ABS_ADDR_kind */
     } value;
@@ -1063,7 +1061,7 @@ DR_API
  * On ARM, either \p index_reg must be #DR_REG_NULL or disp must be 0.
  *
  * On x86, three boolean parameters give control over encoding optimizations
- * (these are ignored on ARM):
+ * (these are ignored on other architectures):
  * - If \p encode_zero_disp, a zero value for disp will not be omitted;
  * - If \p force_full_disp, a small value for disp will not occupy only one byte.
  * - If \p disp_short_addr, short (16-bit for 32-bit mode, 32-bit for
@@ -1132,6 +1130,7 @@ opnd_create_far_base_disp_ex(reg_id_t seg, reg_id_t base_reg, reg_id_t index_reg
                              bool encode_zero_disp, bool force_full_disp,
                              bool disp_short_addr);
 
+#ifdef ARM
 DR_API
 /**
  * Returns a memory reference operand that refers to either a base
@@ -1150,11 +1149,13 @@ DR_API
  * value with #DR_OPND_NEGATED set in opnd_get_flags().
  * Either \p index_reg must be #DR_REG_NULL or disp must be 0.
  *
+ * \note ARM-only.
  */
 opnd_t
 opnd_create_base_disp_arm(reg_id_t base_reg, reg_id_t index_reg,
                           dr_shift_type_t shift_type, uint shift_amount, int disp,
                           dr_opnd_flags_t flags, opnd_size_t size);
+#endif
 
 DR_API
 /**
@@ -1668,13 +1669,15 @@ DR_API
 reg_id_t
 opnd_get_segment(opnd_t opnd);
 
+#ifdef ARM
 DR_API
 /**
  * Assumes \p opnd is a (near or far) base+disp memory reference.
  * Returns DR_SHIFT_NONE if the index register is not shifted.
  * Returns the shift type and \p amount if the index register is shifted (this
  * shift will occur prior to being added to or subtracted from the base
  * register).
+ * \note ARM-only.
  */
 dr_shift_type_t
 opnd_get_index_shift(opnd_t opnd, uint *amount OUT);
@@ -1685,11 +1688,11 @@ DR_API
  * Sets the index register to be shifted by \p amount according to \p shift.
  * Returns whether successful.
  * If the shift amount is out of allowed ranges, returns false.
- * \note On non-ARM platforms where shifted index registers do not exist, this
- * routine will always fail.
+ * \note ARM-only.
  */
 bool
 opnd_set_index_shift(opnd_t *opnd, dr_shift_type_t shift, uint amount);
+#endif /* ARM */
 
 DR_API
 /**
@@ -1989,23 +1992,23 @@ DR_API
 void
 opnd_set_disp(opnd_t *opnd, int disp);
 
+#ifdef X86
 DR_API
 /**
- * Set the displacement and, on x86, the encoding controls of a memory
- * reference operand (the controls are ignored on ARM):
+ * Set the displacement and the encoding controls of a memory
+ * reference operand:
  * - If \p encode_zero_disp, a zero value for \p disp will not be omitted;
  * - If \p force_full_disp, a small value for \p disp will not occupy only one byte.
  * - If \p disp_short_addr, short (16-bit for 32-bit mode, 32-bit for
  *    64-bit mode) addressing will be used (note that this normally only
  *    needs to be specified for an absolute address; otherwise, simply
- *    use the desired short registers for base and/or index).  This is only
- *    honored on x86.
- * On ARM, a negative value for \p disp will be converted into a positive
- * value with #DR_OPND_NEGATED set in opnd_get_flags().
+ *    use the desired short registers for base and/or index).
+ * \note x86-only.
  */
 void
 opnd_set_disp_ex(opnd_t *opnd, int disp, bool encode_zero_disp, bool force_full_disp,
                  bool disp_short_addr);
+#endif
 
 DR_API
 /**

core/arch/opnd_shared.c

@@ -554,25 +554,25 @@ opnd_create_far_base_disp_ex(reg_id_t seg, reg_id_t base_reg, reg_id_t index_reg
         CLIENT_ASSERT(disp == 0 || index_reg == REG_NULL,
                       "opnd_create_*base_disp*: cannot have both disp and index");
     });
+    opnd_set_disp_helper(&opnd, disp);
     opnd.value.base_disp.base_reg = base_reg;
     opnd.value.base_disp.index_reg = index_reg;
-    IF_X86_ELSE({
-        opnd.value.base_disp.scale = (byte) scale;
-    }, {
-        if (scale > 1) {
-            opnd.value.base_disp.shift_type = DR_SHIFT_LSL;
-            opnd.value.base_disp.shift_amount_minus_1 =
-                /* we store the amount minus one */
-                (scale == 2 ? 0 : (scale == 4 ? 1 : 2));
-        } else {
-            opnd.value.base_disp.shift_type = DR_SHIFT_NONE;
-            opnd.value.base_disp.shift_amount_minus_1 = 0;
-        }
-    });
-    opnd_set_disp_helper(&opnd, disp);
+#if defined(ARM)
+    if (scale > 1) {
+        opnd.value.base_disp.shift_type = DR_SHIFT_LSL;
+        opnd.value.base_disp.shift_amount_minus_1 =
+            /* we store the amount minus one */
+            (scale == 2 ? 0 : (scale == 4 ? 1 : 2));
+    } else {
+        opnd.value.base_disp.shift_type = DR_SHIFT_NONE;
+        opnd.value.base_disp.shift_amount_minus_1 = 0;
+    }
+#elif defined(X86)
+    opnd.value.base_disp.scale = (byte) scale;
     opnd.value.base_disp.encode_zero_disp = (byte) encode_zero_disp;
     opnd.value.base_disp.force_full_disp = (byte) force_full_disp;
     opnd.value.base_disp.disp_short_addr = (byte) disp_short_addr;
+#endif
     return opnd;
 }
 
@@ -584,6 +584,7 @@ opnd_create_far_base_disp(reg_id_t seg, reg_id_t base_reg, reg_id_t index_reg, i
                                         false, false, false);
 }
 
+#ifdef ARM
 opnd_t
 opnd_create_base_disp_arm(reg_id_t base_reg, reg_id_t index_reg,
                           dr_shift_type_t shift_type, uint shift_amount, int disp,
@@ -609,11 +610,9 @@ opnd_create_base_disp_arm(reg_id_t base_reg, reg_id_t index_reg,
     opnd.aux.flags = flags;
     if (!opnd_set_index_shift(&opnd, shift_type, shift_amount))
         CLIENT_ASSERT(false, "opnd_create_base_disp_arm: invalid shift type/amount");
-    opnd.value.base_disp.encode_zero_disp = 0;
-    opnd.value.base_disp.force_full_disp = 0;
-    opnd.value.base_disp.disp_short_addr = 0;
     return opnd;
 }
+#endif
 
 #undef opnd_get_base
 #undef opnd_get_disp
@@ -631,6 +630,7 @@ reg_id_t opnd_get_segment(opnd_t opnd) { return OPND_GET_SEGMENT(opnd); }
 #define opnd_get_scale OPND_GET_SCALE
 #define opnd_get_segment OPND_GET_SEGMENT
 
+#ifdef ARM
 dr_shift_type_t
 opnd_get_index_shift(opnd_t opnd, uint *amount OUT)
 {
@@ -696,12 +696,13 @@ opnd_set_index_shift(opnd_t *opnd, dr_shift_type_t shift, uint amount)
     opnd->value.base_disp.shift_type = shift;
     return true;
 }
+#endif /* ARM */
 
 bool
 opnd_is_disp_encode_zero(opnd_t opnd)
 {
     if (opnd_is_base_disp(opnd))
-        return opnd.value.base_disp.encode_zero_disp;
+        return IF_X86_ELSE(opnd.value.base_disp.encode_zero_disp, false);
     CLIENT_ASSERT(false, "opnd_is_disp_encode_zero called on invalid opnd type");
     return false;
 }
@@ -710,7 +711,7 @@ bool
 opnd_is_disp_force_full(opnd_t opnd)
 {
     if (opnd_is_base_disp(opnd))
-        return opnd.value.base_disp.force_full_disp;
+        return IF_X86_ELSE(opnd.value.base_disp.force_full_disp, false);
     CLIENT_ASSERT(false, "opnd_is_disp_force_full called on invalid opnd type");
     return false;
 }
@@ -719,7 +720,7 @@ bool
 opnd_is_disp_short_addr(opnd_t opnd)
 {
     if (opnd_is_base_disp(opnd))
-        return opnd.value.base_disp.disp_short_addr;
+        return IF_X86_ELSE(opnd.value.base_disp.disp_short_addr, false);
     CLIENT_ASSERT(false, "opnd_is_disp_short_addr called on invalid opnd type");
     return false;
 }
@@ -733,6 +734,7 @@ opnd_set_disp(opnd_t *opnd, int disp)
         CLIENT_ASSERT(false, "opnd_set_disp called on invalid opnd type");
 }
 
+#ifdef X86
 void
 opnd_set_disp_ex(opnd_t *opnd, int disp, bool encode_zero_disp, bool force_full_disp,
                  bool disp_short_addr)
@@ -745,6 +747,7 @@ opnd_set_disp_ex(opnd_t *opnd, int disp, bool encode_zero_disp, bool force_full_
     } else
         CLIENT_ASSERT(false, "opnd_set_disp_ex called on invalid opnd type");
 }
+#endif
 
 opnd_t
 opnd_create_abs_addr(void *addr, opnd_size_t data_size)
@@ -1053,7 +1056,10 @@ opnd_replace_reg(opnd_t *opnd, reg_id_t old_reg, reg_id_t new_reg)
                 reg_id_t b = (old_reg == ob) ? new_reg : ob;
                 reg_id_t i = (old_reg == oi) ? new_reg : oi;
                 int d = opnd_get_disp(*opnd);
-#ifdef AARCHXX
+#if defined(AARCH64)
+                /* FIXME i#1569: Include extension and shift. */
+                *opnd = opnd_create_base_disp(b, i, 0, d, size);
+#elif defined(ARM)
                 uint amount;
                 dr_shift_type_t shift = opnd_get_index_shift(*opnd, &amount);
                 dr_opnd_flags_t flags = opnd_get_flags(*opnd);
@@ -1185,24 +1191,25 @@ bool opnd_same(opnd_t op1, opnd_t op2)
         return (IF_X86(op1.aux.segment == op2.aux.segment &&)
                 op1.value.base_disp.base_reg == op2.value.base_disp.base_reg &&
                 op1.value.base_disp.index_reg == op2.value.base_disp.index_reg &&
-                IF_X86_ELSE(op1.value.base_disp.scale ==
-                            op2.value.base_disp.scale &&,
-                            op1.value.base_disp.shift_type ==
-                            op2.value.base_disp.shift_type &&
-                            op1.value.base_disp.shift_amount_minus_1 ==
-                            op2.value.base_disp.shift_amount_minus_1 &&)
+                IF_X86(op1.value.base_disp.scale ==
+                       op2.value.base_disp.scale &&)
+                IF_ARM(op1.value.base_disp.shift_type ==
+                       op2.value.base_disp.shift_type &&
+                       op1.value.base_disp.shift_amount_minus_1 ==
+                       op2.value.base_disp.shift_amount_minus_1 &&)
                 op1.value.base_disp.disp == op2.value.base_disp.disp &&
-                op1.value.base_disp.encode_zero_disp ==
-                op2.value.base_disp.encode_zero_disp &&
-                op1.value.base_disp.force_full_disp ==
-                op2.value.base_disp.force_full_disp &&
-                /* disp_short_addr only matters if no registers are set */
-                (((op1.value.base_disp.base_reg != REG_NULL ||
-                   op1.value.base_disp.index_reg != REG_NULL) &&
-                  (op2.value.base_disp.base_reg != REG_NULL ||
-                   op2.value.base_disp.index_reg != REG_NULL)) ||
-                 op1.value.base_disp.disp_short_addr ==
-                 op2.value.base_disp.disp_short_addr));
+                IF_X86(op1.value.base_disp.encode_zero_disp ==
+                       op2.value.base_disp.encode_zero_disp &&
+                       op1.value.base_disp.force_full_disp ==
+                       op2.value.base_disp.force_full_disp &&
+                       /* disp_short_addr only matters if no registers are set */
+                       (((op1.value.base_disp.base_reg != REG_NULL ||
+                          op1.value.base_disp.index_reg != REG_NULL) &&
+                         (op2.value.base_disp.base_reg != REG_NULL ||
+                          op2.value.base_disp.index_reg != REG_NULL)) ||
+                        op1.value.base_disp.disp_short_addr ==
+                        op2.value.base_disp.disp_short_addr) &&)
+                true);
 #if defined(X64) || defined(ARM)
     case REL_ADDR_kind:
 #endif
@@ -1786,18 +1793,21 @@ opnd_compute_address_priv(opnd_t opnd, priv_mcontext_t *mc)
     ptr_int_t scaled_index = 0;
     if (opnd_is_base_disp(opnd)) {
         reg_id_t index = opnd_get_index(opnd);
-#ifdef X86
+#if defined(X86)
         ptr_int_t scale = opnd_get_scale(opnd);
         scaled_index = scale * reg_get_value_priv(index, mc);
-#elif defined(AARCHXX)
+#elif defined(AARCH64)
+        reg_t index_val = reg_get_value_priv(index, mc);
+        /* FIXME i#1569: Compute extension and shift. */
+        scaled_index = index_val;
+#elif defined(ARM)
         uint amount;
         dr_shift_type_t type = opnd_get_index_shift(opnd, &amount);
         reg_t index_val = reg_get_value_priv(index, mc);
         switch (type) {
         case DR_SHIFT_LSL:
             scaled_index = index_val << amount;
             break;
-# ifndef AARCH64
         case DR_SHIFT_LSR:
             scaled_index = index_val >> amount;
             break;
@@ -1812,7 +1822,6 @@ opnd_compute_address_priv(opnd_t opnd, priv_mcontext_t *mc)
             scaled_index = (index_val >> 1) ||
                 (TEST(EFLAGS_C, mc->cpsr) ? (1 << (sizeof(reg_t)*8-1)) : 0);
             break;
-# endif
         default:
             scaled_index = index_val;
         }