diff mbox

[RFA,6/6] Remove long_long_align_bit gdbarch attribute

Message ID 20180424152222.8053-7-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey April 24, 2018, 3:22 p.m. UTC 
  This removes the long_long_align_bit gdbarch attribute in favor of
type_align.  This uncovered two possible issues.

First, arc-tdep.c claimed that long long alignment was 32 bits, but
gcc's arc.h says:

    #define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
    (TYPE_MODE (strip_array_types (TYPE)) == DFmode \
     ? MIN ((COMPUTED), 32) : (COMPUTED))

Here, DFmode means "double".  So, I've implemented arc_type_align
according to this.  I do not have a good way to test this.

Second, jit.c, the sole user of long_long_align_bit, was confusing
"long long" with uint64_t.  The relevant structure is defined in the
JIT API part of the manual as:

     struct jit_code_entry
     {
       struct jit_code_entry *next_entry;
       struct jit_code_entry *prev_entry;
       const char *symfile_addr;
       uint64_t symfile_size;
     };

I've changed this code to use uint64_t.

2018-04-24  Tom Tromey  <tom@tromey.com>

	* jit.c (jit_read_code_entry): Use type_align.
	* i386-tdep.c (i386_gdbarch_init): Don't call
	set_gdbarch_long_long_align_bit.
	* gdbarch.sh: Remove long_long_align_bit.
	* gdbarch.c, gdbarch.h: Rebuild.
	* arc-tdep.c (arc_type_align): New function.
	(arc_gdbarch_init): Use arc_type_align.  Don't call
	set_gdbarch_long_long_align_bit.
---
 gdb/ChangeLog   | 11 +++++++++++
 gdb/arc-tdep.c  | 15 ++++++++++++++-
 gdb/gdbarch.c   | 23 -----------------------
 gdb/gdbarch.h   |  6 ------
 gdb/gdbarch.sh  |  3 ---
 gdb/i386-tdep.c |  1 -
 gdb/jit.c       |  4 ++--
 7 files changed, 27 insertions(+), 36 deletions(-)

Comments

Tom Tromey April 24, 2018, 3:24 p.m. UTC | #1 
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This removes the long_long_align_bit gdbarch attribute in favor of
Tom> type_align.  This uncovered two possible issues.

Tom> First, arc-tdep.c claimed that long long alignment was 32 bits, but
Tom> gcc's arc.h says:

Tom>     #define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
Tom>     (TYPE_MODE (strip_array_types (TYPE)) == DFmode \
Tom>      ? MIN ((COMPUTED), 32) : (COMPUTED))

Tom> Here, DFmode means "double".  So, I've implemented arc_type_align
Tom> according to this.  I do not have a good way to test this.

Anton, could you take a look at this part of the patch?

Tom
Anton Kolesov April 24, 2018, 5:16 p.m. UTC | #2 
> First, arc-tdep.c claimed that long long alignment was 32 bits, but gcc's arc.h
> says:
> 
>     #define ADJUST_FIELD_ALIGN(FIELD, TYPE, COMPUTED) \
>     (TYPE_MODE (strip_array_types (TYPE)) == DFmode \
>      ? MIN ((COMPUTED), 32) : (COMPUTED))
> 
> Here, DFmode means "double".  So, I've implemented arc_type_align
> according to this.  I do not have a good way to test this.
> 
> diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c index b0d51addd3..86cf522443
> 100644
> --- a/gdb/arc-tdep.c
> +++ b/gdb/arc-tdep.c
> @@ -1957,6 +1957,19 @@ arc_tdesc_init (struct gdbarch_info info, const
> struct target_desc **tdesc,
>    return TRUE;
>  }
> 
> +/* Implement the type_align gdbarch function.  */
> +
> +static ULONGEST
> +arc_type_align (struct gdbarch *gdbarch, struct type *type) {
> +  type = check_typedef (type);
> +
> +  if (TYPE_CODE (type) == TYPE_CODE_FLT && TYPE_LENGTH (type) == 8)
> +    return 4;
> +
> +  return TYPE_LENGTH (type);
> +}
> +

Hi Tom,

If I read this correctly, then it would set 4 byte alignment only to 8 byte
floating point type? That doesn't look right, because on ARC there is never
more than 4 byte alignment for any data type. GCC arc.h has:

    #define BIGGEST_ALIGNMENT 32

According to comments it used to be 64, but was changed to 32 at some
undefined point in the past.

I would have implemented this as:

static ULONGEST
arc_type_align (struct gdbarch *gdbarch, struct type *type) {
  type = check_typedef (type);
  return std::min<ULONGEST>(4, TYPE_LENGTH (type));
}

Anton
Tom Tromey April 26, 2018, 8:56 p.m. UTC | #3 
>>>>> "Anton" == Anton Kolesov <Anton.Kolesov@synopsys.com> writes:

Anton> I would have implemented this as:

Anton> static ULONGEST
Anton> arc_type_align (struct gdbarch *gdbarch, struct type *type) {
Anton>   type = check_typedef (type);
Anton>   return std::min<ULONGEST>(4, TYPE_LENGTH (type));
Anton> }

Thanks, I've made this change locally.

Tom
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 04920fbc4b..5e59d4e647 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@ 
+2018-04-24  Tom Tromey  <tom@tromey.com>
+
+	* jit.c (jit_read_code_entry): Use type_align.
+	* i386-tdep.c (i386_gdbarch_init): Don't call
+	set_gdbarch_long_long_align_bit.
+	* gdbarch.sh: Remove long_long_align_bit.
+	* gdbarch.c, gdbarch.h: Rebuild.
+	* arc-tdep.c (arc_type_align): New function.
+	(arc_gdbarch_init): Use arc_type_align.  Don't call
+	set_gdbarch_long_long_align_bit.
+
 2018-04-24  Tom Tromey  <tom@tromey.com>
 
 	* rust-lang.c (rust_type_alignment): Remove.
diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index b0d51addd3..86cf522443 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -1957,6 +1957,19 @@  arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
   return TRUE;
 }
 
+/* Implement the type_align gdbarch function.  */
+
+static ULONGEST
+arc_type_align (struct gdbarch *gdbarch, struct type *type)
+{
+  type = check_typedef (type);
+
+  if (TYPE_CODE (type) == TYPE_CODE_FLT && TYPE_LENGTH (type) == 8)
+    return 4;
+
+  return TYPE_LENGTH (type);
+}
+
 /* Implement the "init" gdbarch method.  */
 
 static struct gdbarch *
@@ -1982,7 +1995,7 @@  arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_int_bit (gdbarch, 32);
   set_gdbarch_long_bit (gdbarch, 32);
   set_gdbarch_long_long_bit (gdbarch, 64);
-  set_gdbarch_long_long_align_bit (gdbarch, 32);
+  set_gdbarch_type_align (gdbarch, arc_type_align);
   set_gdbarch_float_bit (gdbarch, 32);
   set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
   set_gdbarch_double_bit (gdbarch, 64);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index dd7c89d948..82ac751913 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -175,7 +175,6 @@  struct gdbarch
   int int_bit;
   int long_bit;
   int long_long_bit;
-  int long_long_align_bit;
   int half_bit;
   const struct floatformat ** half_format;
   int float_bit;
@@ -389,7 +388,6 @@  gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->int_bit = 4*TARGET_CHAR_BIT;
   gdbarch->long_bit = 4*TARGET_CHAR_BIT;
   gdbarch->long_long_bit = 2*gdbarch->long_bit;
-  gdbarch->long_long_align_bit = 2*gdbarch->long_bit;
   gdbarch->half_bit = 2*TARGET_CHAR_BIT;
   gdbarch->float_bit = 4*TARGET_CHAR_BIT;
   gdbarch->double_bit = 8*TARGET_CHAR_BIT;
@@ -530,7 +528,6 @@  verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of int_bit, invalid_p == 0 */
   /* Skip verify of long_bit, invalid_p == 0 */
   /* Skip verify of long_long_bit, invalid_p == 0 */
-  /* Skip verify of long_long_align_bit, invalid_p == 0 */
   /* Skip verify of half_bit, invalid_p == 0 */
   if (gdbarch->half_format == 0)
     gdbarch->half_format = floatformats_ieee_half;
@@ -1165,9 +1162,6 @@  gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   fprintf_unfiltered (file,
                       "gdbarch_dump: long_double_format = %s\n",
                       pformat (gdbarch->long_double_format));
-  fprintf_unfiltered (file,
-                      "gdbarch_dump: long_long_align_bit = %s\n",
-                      plongest (gdbarch->long_long_align_bit));
   fprintf_unfiltered (file,
                       "gdbarch_dump: long_long_bit = %s\n",
                       plongest (gdbarch->long_long_bit));
@@ -1638,23 +1632,6 @@  set_gdbarch_long_long_bit (struct gdbarch *gdbarch,
   gdbarch->long_long_bit = long_long_bit;
 }
 
-int
-gdbarch_long_long_align_bit (struct gdbarch *gdbarch)
-{
-  gdb_assert (gdbarch != NULL);
-  /* Skip verify of long_long_align_bit, invalid_p == 0 */
-  if (gdbarch_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "gdbarch_long_long_align_bit called\n");
-  return gdbarch->long_long_align_bit;
-}
-
-void
-set_gdbarch_long_long_align_bit (struct gdbarch *gdbarch,
-                                 int long_long_align_bit)
-{
-  gdbarch->long_long_align_bit = long_long_align_bit;
-}
-
 int
 gdbarch_half_bit (struct gdbarch *gdbarch)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 3848ec50c7..b3a15c9898 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -147,12 +147,6 @@  extern void set_gdbarch_long_bit (struct gdbarch *gdbarch, int long_bit);
 extern int gdbarch_long_long_bit (struct gdbarch *gdbarch);
 extern void set_gdbarch_long_long_bit (struct gdbarch *gdbarch, int long_long_bit);
 
-/* Alignment of a long long or unsigned long long for the target
-   machine. */
-
-extern int gdbarch_long_long_align_bit (struct gdbarch *gdbarch);
-extern void set_gdbarch_long_long_align_bit (struct gdbarch *gdbarch, int long_long_align_bit);
-
 /* The ABI default bit-size and format for "half", "float", "double", and
    "long double".  These bit/format pairs should eventually be combined
    into a single object.  For the moment, just initialize them as a pair.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index bb62e6d620..bf12c68487 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -360,9 +360,6 @@  v;int;long_bit;;;8 * sizeof (long);4*TARGET_CHAR_BIT;;0
 # Number of bits in a long long or unsigned long long for the target
 # machine.
 v;int;long_long_bit;;;8 * sizeof (LONGEST);2*gdbarch->long_bit;;0
-# Alignment of a long long or unsigned long long for the target
-# machine.
-v;int;long_long_align_bit;;;8 * sizeof (LONGEST);2*gdbarch->long_bit;;0
 
 # The ABI default bit-size and format for "half", "float", "double", and
 # "long double".  These bit/format pairs should eventually be combined
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index e1938304bb..9b6a770a7a 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8431,7 +8431,6 @@  i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   tdep->record_regmap = i386_record_regmap;
 
-  set_gdbarch_long_long_align_bit (gdbarch, 32);
   set_gdbarch_type_align (gdbarch, i386_type_align);
 
   /* The format used for `long double' on almost all i386 targets is
diff --git a/gdb/jit.c b/gdb/jit.c
index 2f23c058a0..8cd645cb66 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -413,8 +413,8 @@  jit_read_code_entry (struct gdbarch *gdbarch,
   ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
   ptr_size = TYPE_LENGTH (ptr_type);
 
-  /* Figure out where the longlong value will be.  */
-  align_bytes = gdbarch_long_long_align_bit (gdbarch) / 8;
+  /* Figure out where the uint64_t value will be.  */
+  align_bytes = type_align (builtin_type (gdbarch)->builtin_uint64);
   off = 3 * ptr_size;
   off = (off + (align_bytes - 1)) & ~(align_bytes - 1);