[3/3] gdb/nds32: Use type_align instead of nds32_type_align
Commit Message
The general type_align method should be a suitable alternative to
nds32_type_align, so switch to use that.
The only change this will introduce is related to static fields in a
struct or union, the existing code doesn't take account of static
fields when computing the alignment for structs of unions, though this
is probably a bug - which would probably be exposed by the test case
gdb.cp/many-args.exp, though I don't have any way to test this target
right now.
gdb/ChangeLog:
* nds32-tdep.c (nds32_type_align): Delete.
(nds32_push_dummy_call): Use type_align instead.
---
gdb/ChangeLog | 5 +++++
gdb/nds32-tdep.c | 49 ++-----------------------------------------------
2 files changed, 7 insertions(+), 47 deletions(-)
Comments
On Sat, 13 Apr 2019 00:25:34 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> The general type_align method should be a suitable alternative to
> nds32_type_align, so switch to use that.
>
> The only change this will introduce is related to static fields in a
> struct or union, the existing code doesn't take account of static
> fields when computing the alignment for structs of unions, though this
> is probably a bug - which would probably be exposed by the test case
> gdb.cp/many-args.exp, though I don't have any way to test this target
> right now.
>
> gdb/ChangeLog:
>
> * nds32-tdep.c (nds32_type_align): Delete.
> (nds32_push_dummy_call): Use type_align instead.
I think that nds32_type_align needs to be registered in
nds32_gdbarch_init().
Otherwise, LGTM.
Kevin
* Kevin Buettner <kevinb@redhat.com> [2019-04-14 11:53:52 -0700]:
> On Sat, 13 Apr 2019 00:25:34 +0100
> Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>
> > The general type_align method should be a suitable alternative to
> > nds32_type_align, so switch to use that.
> >
> > The only change this will introduce is related to static fields in a
> > struct or union, the existing code doesn't take account of static
> > fields when computing the alignment for structs of unions, though this
> > is probably a bug - which would probably be exposed by the test case
> > gdb.cp/many-args.exp, though I don't have any way to test this target
> > right now.
> >
> > gdb/ChangeLog:
> >
> > * nds32-tdep.c (nds32_type_align): Delete.
> > (nds32_push_dummy_call): Use type_align instead.
>
> I think that nds32_type_align needs to be registered in
> nds32_gdbarch_init().
No, I deleted nds32_type_align completely. It doesn't have any
special vector type handling, so the default type_align should be
fine.
The one change that will be seen is that the old nds32_type_align
counts static fields within structs when computing the alignment. I
haven't tried any testing, but I would guess this was a bug. Unless
someone has the ability to test the target I'll probably just push
this change, and if it turns out the static field handling is wrong,
then it's easy enough to fix later.
Thanks,
Andrew
>
> Otherwise, LGTM.
>
> Kevin
On Wed, 17 Apr 2019 21:59:46 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> * Kevin Buettner <kevinb@redhat.com> [2019-04-14 11:53:52 -0700]:
>
> > On Sat, 13 Apr 2019 00:25:34 +0100
> > Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> >
> > > The general type_align method should be a suitable alternative to
> > > nds32_type_align, so switch to use that.
> > >
> > > The only change this will introduce is related to static fields in a
> > > struct or union, the existing code doesn't take account of static
> > > fields when computing the alignment for structs of unions, though this
> > > is probably a bug - which would probably be exposed by the test case
> > > gdb.cp/many-args.exp, though I don't have any way to test this target
> > > right now.
> > >
> > > gdb/ChangeLog:
> > >
> > > * nds32-tdep.c (nds32_type_align): Delete.
> > > (nds32_push_dummy_call): Use type_align instead.
> >
> > I think that nds32_type_align needs to be registered in
> > nds32_gdbarch_init().
>
> No, I deleted nds32_type_align completely. It doesn't have any
> special vector type handling, so the default type_align should be
> fine.
I see. This is okay then. (Sorry for not reading it more closely.)
> The one change that will be seen is that the old nds32_type_align
> counts static fields within structs when computing the alignment. I
> haven't tried any testing, but I would guess this was a bug. Unless
> someone has the ability to test the target I'll probably just push
> this change, and if it turns out the static field handling is wrong,
> then it's easy enough to fix later.
I think that's fine.
Kevin
@@ -1439,51 +1439,6 @@ nds32_check_calling_use_fpr (struct type *type)
return typecode == TYPE_CODE_FLT;
}
-/* Return the alignment (in bytes) of the given type. */
-
-static int
-nds32_type_align (struct type *type)
-{
- int n;
- int align;
- int falign;
-
- type = check_typedef (type);
- switch (TYPE_CODE (type))
- {
- default:
- /* Should never happen. */
- internal_error (__FILE__, __LINE__, _("unknown type alignment"));
- return 4;
-
- case TYPE_CODE_PTR:
- case TYPE_CODE_ENUM:
- case TYPE_CODE_INT:
- case TYPE_CODE_FLT:
- case TYPE_CODE_SET:
- case TYPE_CODE_RANGE:
- case TYPE_CODE_REF:
- case TYPE_CODE_CHAR:
- case TYPE_CODE_BOOL:
- return TYPE_LENGTH (type);
-
- case TYPE_CODE_ARRAY:
- case TYPE_CODE_COMPLEX:
- return nds32_type_align (TYPE_TARGET_TYPE (type));
-
- case TYPE_CODE_STRUCT:
- case TYPE_CODE_UNION:
- align = 1;
- for (n = 0; n < TYPE_NFIELDS (type); n++)
- {
- falign = nds32_type_align (TYPE_FIELD_TYPE (type, n));
- if (falign > align)
- align = falign;
- }
- return align;
- }
-}
-
/* Implement the "push_dummy_call" gdbarch method. */
static CORE_ADDR
@@ -1522,7 +1477,7 @@ nds32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
for (i = 0; i < nargs; i++)
{
struct type *type = value_type (args[i]);
- int align = nds32_type_align (type);
+ int align = type_align (type);
/* If align is zero, it may be an empty struct.
Just ignore the argument of empty struct. */
@@ -1548,7 +1503,7 @@ nds32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
type = value_type (args[i]);
calling_use_fpr = nds32_check_calling_use_fpr (type);
len = TYPE_LENGTH (type);
- align = nds32_type_align (type);
+ align = type_align (type);
val = value_contents (args[i]);
/* The size of a composite type larger than 4 bytes will be rounded