* [PATCH 0/3] RISC-V: gdb.base/gnu_vector fixes. @ 2018-11-06 21:43 Jim Wilson 2018-11-06 21:44 ` [PATCH 3/3] " Jim Wilson ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Jim Wilson @ 2018-11-06 21:43 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess This set of patches fixes these infcall related failures on a rv64gc linux system. FAIL: gdb.base/gnu_vector.exp: call add_many_charvecs FAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs The first one fails because we have 2 4-byte vector args passed in the same 8-byte stack slot. The code is failing round up the address of a stack slot when an arg is smaller than the stack slot size. The second one fails because of two problems. The 16-byte vector of floats is given 4 byte alignment when it should have 16 byte alignment. This is because there is no support for vector types. And unnamed args that require twice XLEN alignment are supposed to be passed in aligned register pairs. The support for this is also missing. This was tested on a rv64gc linux system with the gdb testsuite, and it fixes 2 failures without causing any regressions. Jim ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-06 21:43 [PATCH 0/3] RISC-V: gdb.base/gnu_vector fixes Jim Wilson @ 2018-11-06 21:44 ` Jim Wilson 2018-11-08 14:34 ` Andrew Burgess 2018-11-06 21:44 ` [PATCH 1/3] " Jim Wilson 2018-11-06 21:44 ` [PATCH 2/3] " Jim Wilson 2 siblings, 1 reply; 9+ messages in thread From: Jim Wilson @ 2018-11-06 21:44 UTC (permalink / raw) To: gdb-patches; +Cc: Jim Wilson Unnamed arguments with 2*XLEN alignment are passed in aligned register pairs. gdb/ * riscv-tdep.c (struct riscv_arg_info): New field is_unnamed. (riscv_call_arg_scalar_int): If unnamed arg with twice xlen alignment, then increment next_regnum if odd. (riscv_arg_location): New arg is_unnamed. Set ainfo->is_unnamed. (riscv_push_dummy_call): New local ftype. Call check_typedef to set function type. Pass new arg to riscv_arg_location based on function type. (riscv_return_value): Pass new arg to riscv_arg_location. --- gdb/riscv-tdep.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 3d4f7e3dcc..93310c329f 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -1737,6 +1737,9 @@ struct riscv_arg_info then this offset will be set to 0. */ int c_offset; } argloc[2]; + + /* TRUE if this is an unnamed argument. */ + bool is_unnamed; }; /* Information about a set of registers being used for passing arguments as @@ -1932,6 +1935,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, { int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length; + /* Unnamed arguments in registers that require 2*XLEN alignment are + passed in an aligned register pair. */ + if (ainfo->is_unnamed && (ainfo->align == cinfo->xlen * 2) + && cinfo->int_regs.next_regnum & 1) + cinfo->int_regs.next_regnum++; + if (!riscv_assign_reg_location (&ainfo->argloc[0], &cinfo->int_regs, len, 0)) riscv_assign_stack_location (&ainfo->argloc[0], @@ -2222,11 +2231,12 @@ static void riscv_arg_location (struct gdbarch *gdbarch, struct riscv_arg_info *ainfo, struct riscv_call_info *cinfo, - struct type *type) + struct type *type, bool is_unnamed) { ainfo->type = type; ainfo->length = TYPE_LENGTH (ainfo->type); ainfo->align = riscv_type_alignment (ainfo->type); + ainfo->is_unnamed = is_unnamed; ainfo->contents = nullptr; switch (TYPE_CODE (ainfo->type)) @@ -2375,6 +2385,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, CORE_ADDR osp = sp; + struct type *ftype = check_typedef (value_type (function)); + + if (TYPE_CODE (ftype) == TYPE_CODE_PTR) + ftype = check_typedef (TYPE_TARGET_TYPE (ftype)); + /* We'll use register $a0 if we're returning a struct. */ if (struct_return) ++call_info.int_regs.next_regnum; @@ -2388,7 +2403,8 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, arg_value = args[i]; arg_type = check_typedef (value_type (arg_value)); - riscv_arg_location (gdbarch, info, &call_info, arg_type); + riscv_arg_location (gdbarch, info, &call_info, arg_type, + TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype)); if (info->type != arg_type) arg_value = value_cast (info->type, arg_value); @@ -2565,7 +2581,7 @@ riscv_return_value (struct gdbarch *gdbarch, struct type *arg_type; arg_type = check_typedef (type); - riscv_arg_location (gdbarch, &info, &call_info, arg_type); + riscv_arg_location (gdbarch, &info, &call_info, arg_type, false); if (riscv_debug_infcall > 0) { -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-06 21:44 ` [PATCH 3/3] " Jim Wilson @ 2018-11-08 14:34 ` Andrew Burgess 2018-11-08 14:44 ` Andrew Burgess 0 siblings, 1 reply; 9+ messages in thread From: Andrew Burgess @ 2018-11-08 14:34 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches * Jim Wilson <jimw@sifive.com> [2018-11-06 13:44:46 -0800]: > Unnamed arguments with 2*XLEN alignment are passed in aligned register pairs. > > gdb/ > * riscv-tdep.c (struct riscv_arg_info): New field is_unnamed. > (riscv_call_arg_scalar_int): If unnamed arg with twice xlen alignment, > then increment next_regnum if odd. > (riscv_arg_location): New arg is_unnamed. Set ainfo->is_unnamed. > (riscv_push_dummy_call): New local ftype. Call check_typedef to set > function type. Pass new arg to riscv_arg_location based on function > type. > (riscv_return_value): Pass new arg to riscv_arg_location. Could you change the titles for the 3 commits in this series please to make them more descriptive of the fix in each commit. This also avoids having 3 commits in a row with the same title line, which can look like a mistake in 'git log --format=oneline'. > --- > gdb/riscv-tdep.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 3d4f7e3dcc..93310c329f 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -1737,6 +1737,9 @@ struct riscv_arg_info > then this offset will be set to 0. */ > int c_offset; > } argloc[2]; > + > + /* TRUE if this is an unnamed argument. */ > + bool is_unnamed; > }; > > /* Information about a set of registers being used for passing arguments as > @@ -1932,6 +1935,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > { > int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length; > > + /* Unnamed arguments in registers that require 2*XLEN alignment are > + passed in an aligned register pair. */ > + if (ainfo->is_unnamed && (ainfo->align == cinfo->xlen * 2) > + && cinfo->int_regs.next_regnum & 1) > + cinfo->int_regs.next_regnum++; > + > if (!riscv_assign_reg_location (&ainfo->argloc[0], > &cinfo->int_regs, len, 0)) > riscv_assign_stack_location (&ainfo->argloc[0], > @@ -2222,11 +2231,12 @@ static void > riscv_arg_location (struct gdbarch *gdbarch, > struct riscv_arg_info *ainfo, > struct riscv_call_info *cinfo, > - struct type *type) > + struct type *type, bool is_unnamed) Could you update the functions header comment to explain what impact IS_UNNAMED will have on the behaviour of the function. > { > ainfo->type = type; > ainfo->length = TYPE_LENGTH (ainfo->type); > ainfo->align = riscv_type_alignment (ainfo->type); > + ainfo->is_unnamed = is_unnamed; > ainfo->contents = nullptr; > > switch (TYPE_CODE (ainfo->type)) > @@ -2375,6 +2385,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, > > CORE_ADDR osp = sp; > > + struct type *ftype = check_typedef (value_type (function)); > + > + if (TYPE_CODE (ftype) == TYPE_CODE_PTR) > + ftype = check_typedef (TYPE_TARGET_TYPE (ftype)); > + > /* We'll use register $a0 if we're returning a struct. */ > if (struct_return) > ++call_info.int_regs.next_regnum; > @@ -2388,7 +2403,8 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, > arg_value = args[i]; > arg_type = check_typedef (value_type (arg_value)); > > - riscv_arg_location (gdbarch, info, &call_info, arg_type); > + riscv_arg_location (gdbarch, info, &call_info, arg_type, > + TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype)); > > if (info->type != arg_type) > arg_value = value_cast (info->type, arg_value); > @@ -2565,7 +2581,7 @@ riscv_return_value (struct gdbarch *gdbarch, > struct type *arg_type; > > arg_type = check_typedef (type); > - riscv_arg_location (gdbarch, &info, &call_info, arg_type); > + riscv_arg_location (gdbarch, &info, &call_info, arg_type, false); > > if (riscv_debug_infcall > 0) > { > -- > 2.17.1 > Otherwise looks fine. Thanks, Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-08 14:34 ` Andrew Burgess @ 2018-11-08 14:44 ` Andrew Burgess 0 siblings, 0 replies; 9+ messages in thread From: Andrew Burgess @ 2018-11-08 14:44 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches * Andrew Burgess <andrew.burgess@embecosm.com> [2018-11-08 14:34:11 +0000]: > * Jim Wilson <jimw@sifive.com> [2018-11-06 13:44:46 -0800]: > > > Unnamed arguments with 2*XLEN alignment are passed in aligned register pairs. > > > > gdb/ > > * riscv-tdep.c (struct riscv_arg_info): New field is_unnamed. > > (riscv_call_arg_scalar_int): If unnamed arg with twice xlen alignment, > > then increment next_regnum if odd. > > (riscv_arg_location): New arg is_unnamed. Set ainfo->is_unnamed. > > (riscv_push_dummy_call): New local ftype. Call check_typedef to set > > function type. Pass new arg to riscv_arg_location based on function > > type. > > (riscv_return_value): Pass new arg to riscv_arg_location. > > Could you change the titles for the 3 commits in this series please to > make them more descriptive of the fix in each commit. This also > avoids having 3 commits in a row with the same title line, which can > look like a mistake in 'git log --format=oneline'. Also can you make sure the commit message for each patch includes details about which tests you expect to go from fail to pass, and on which RISC-V variants you expect to see improvements. Thanks, Andrew > > > > > > > --- > > gdb/riscv-tdep.c | 22 +++++++++++++++++++--- > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > > index 3d4f7e3dcc..93310c329f 100644 > > --- a/gdb/riscv-tdep.c > > +++ b/gdb/riscv-tdep.c > > @@ -1737,6 +1737,9 @@ struct riscv_arg_info > > then this offset will be set to 0. */ > > int c_offset; > > } argloc[2]; > > + > > + /* TRUE if this is an unnamed argument. */ > > + bool is_unnamed; > > }; > > > > /* Information about a set of registers being used for passing arguments as > > @@ -1932,6 +1935,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > > { > > int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length; > > > > + /* Unnamed arguments in registers that require 2*XLEN alignment are > > + passed in an aligned register pair. */ > > + if (ainfo->is_unnamed && (ainfo->align == cinfo->xlen * 2) > > + && cinfo->int_regs.next_regnum & 1) > > + cinfo->int_regs.next_regnum++; > > + > > if (!riscv_assign_reg_location (&ainfo->argloc[0], > > &cinfo->int_regs, len, 0)) > > riscv_assign_stack_location (&ainfo->argloc[0], > > @@ -2222,11 +2231,12 @@ static void > > riscv_arg_location (struct gdbarch *gdbarch, > > struct riscv_arg_info *ainfo, > > struct riscv_call_info *cinfo, > > - struct type *type) > > + struct type *type, bool is_unnamed) > > Could you update the functions header comment to explain what impact > IS_UNNAMED will have on the behaviour of the function. > > > { > > ainfo->type = type; > > ainfo->length = TYPE_LENGTH (ainfo->type); > > ainfo->align = riscv_type_alignment (ainfo->type); > > + ainfo->is_unnamed = is_unnamed; > > ainfo->contents = nullptr; > > > > switch (TYPE_CODE (ainfo->type)) > > @@ -2375,6 +2385,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, > > > > CORE_ADDR osp = sp; > > > > + struct type *ftype = check_typedef (value_type (function)); > > + > > + if (TYPE_CODE (ftype) == TYPE_CODE_PTR) > > + ftype = check_typedef (TYPE_TARGET_TYPE (ftype)); > > + > > /* We'll use register $a0 if we're returning a struct. */ > > if (struct_return) > > ++call_info.int_regs.next_regnum; > > @@ -2388,7 +2403,8 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, > > arg_value = args[i]; > > arg_type = check_typedef (value_type (arg_value)); > > > > - riscv_arg_location (gdbarch, info, &call_info, arg_type); > > + riscv_arg_location (gdbarch, info, &call_info, arg_type, > > + TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype)); > > > > if (info->type != arg_type) > > arg_value = value_cast (info->type, arg_value); > > @@ -2565,7 +2581,7 @@ riscv_return_value (struct gdbarch *gdbarch, > > struct type *arg_type; > > > > arg_type = check_typedef (type); > > - riscv_arg_location (gdbarch, &info, &call_info, arg_type); > > + riscv_arg_location (gdbarch, &info, &call_info, arg_type, false); > > > > if (riscv_debug_infcall > 0) > > { > > -- > > 2.17.1 > > > > Otherwise looks fine. > > Thanks, > Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-06 21:43 [PATCH 0/3] RISC-V: gdb.base/gnu_vector fixes Jim Wilson 2018-11-06 21:44 ` [PATCH 3/3] " Jim Wilson @ 2018-11-06 21:44 ` Jim Wilson 2018-11-08 15:43 ` Andrew Burgess 2018-11-06 21:44 ` [PATCH 2/3] " Jim Wilson 2 siblings, 1 reply; 9+ messages in thread From: Jim Wilson @ 2018-11-06 21:44 UTC (permalink / raw) To: gdb-patches; +Cc: Jim Wilson Ensure that stack slots are always the same size as XLEN by rounding up arg sizes when computing the address of the next stack slot. gdb/ * riscv-tdep.c (riscv_assign_stack_location): New arg slot_align. Use with align_up when setting arg_offset. (riscv_call_arg_scalar_int): Call riscv_assign_stack_location with cinfo->xlen as new arg. --- gdb/riscv-tdep.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index db372e2163..ac4f2533f4 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -1868,13 +1868,13 @@ riscv_assign_reg_location (struct riscv_arg_info::location *loc, static void riscv_assign_stack_location (struct riscv_arg_info::location *loc, struct riscv_memory_offsets *memory, - int length, int align) + int length, int align, int slot_align) { loc->loc_type = riscv_arg_info::location::on_stack; memory->arg_offset = align_up (memory->arg_offset, align); loc->loc_data.offset = memory->arg_offset; - memory->arg_offset += length; + memory->arg_offset += align_up (length, slot_align); loc->c_length = length; /* Offset is always 0, either we're the first location part, in which @@ -1919,7 +1919,7 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, cinfo->xlen, 0)) riscv_assign_stack_location (&ainfo->argloc[1], &cinfo->memory, cinfo->xlen, - cinfo->xlen); + cinfo->xlen, cinfo->xlen); } else { @@ -1928,7 +1928,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, if (!riscv_assign_reg_location (&ainfo->argloc[0], &cinfo->int_regs, len, 0)) riscv_assign_stack_location (&ainfo->argloc[0], - &cinfo->memory, len, ainfo->align); + &cinfo->memory, len, ainfo->align, + cinfo->xlen); if (len < ainfo->length) { @@ -1937,7 +1938,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, &cinfo->int_regs, len, cinfo->xlen)) riscv_assign_stack_location (&ainfo->argloc[1], - &cinfo->memory, len, cinfo->xlen); + &cinfo->memory, len, cinfo->xlen, + cinfo->xlen); } } } -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-06 21:44 ` [PATCH 1/3] " Jim Wilson @ 2018-11-08 15:43 ` Andrew Burgess 2018-11-13 1:52 ` Jim Wilson 0 siblings, 1 reply; 9+ messages in thread From: Andrew Burgess @ 2018-11-08 15:43 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches * Jim Wilson <jimw@sifive.com> [2018-11-06 13:44:03 -0800]: > Ensure that stack slots are always the same size as XLEN by rounding up arg > sizes when computing the address of the next stack slot. > > gdb/ > * riscv-tdep.c (riscv_assign_stack_location): New arg slot_align. > Use with align_up when setting arg_offset. > (riscv_call_arg_scalar_int): Call riscv_assign_stack_location with > cinfo->xlen as new arg. > --- > gdb/riscv-tdep.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index db372e2163..ac4f2533f4 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -1868,13 +1868,13 @@ riscv_assign_reg_location (struct riscv_arg_info::location *loc, > static void > riscv_assign_stack_location (struct riscv_arg_info::location *loc, > struct riscv_memory_offsets *memory, > - int length, int align) > + int length, int align, int slot_align) I struggled to understand what the difference between align and slot_align is in this patch, it feels like.... > { > loc->loc_type = riscv_arg_info::location::on_stack; > memory->arg_offset > = align_up (memory->arg_offset, align); > loc->loc_data.offset = memory->arg_offset; > - memory->arg_offset += length; > + memory->arg_offset += align_up (length, slot_align); > loc->c_length = length; > > /* Offset is always 0, either we're the first location part, in which > @@ -1919,7 +1919,7 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > cinfo->xlen, 0)) > riscv_assign_stack_location (&ainfo->argloc[1], > &cinfo->memory, cinfo->xlen, > - cinfo->xlen); > + cinfo->xlen, cinfo->xlen); > } > else > { > @@ -1928,7 +1928,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > if (!riscv_assign_reg_location (&ainfo->argloc[0], > &cinfo->int_regs, len, 0)) > riscv_assign_stack_location (&ainfo->argloc[0], > - &cinfo->memory, len, ainfo->align); > + &cinfo->memory, len, ainfo->align, > + cinfo->xlen); .... you might be better off just passing a better value of align through here. Testing on gdb.base/gnu_vector.exp shows that doing this fixes the same problem that your original patch fixes. My revision is below, but I've only just kicked off a test run, so there might be problems I'm not seeing. Thanks, Andrew > > if (len < ainfo->length) > { > @@ -1937,7 +1938,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > &cinfo->int_regs, len, > cinfo->xlen)) > riscv_assign_stack_location (&ainfo->argloc[1], > - &cinfo->memory, len, cinfo->xlen); > + &cinfo->memory, len, cinfo->xlen, > + cinfo->xlen); > } > } > } > -- > 2.17.1 > --- diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index a0b2d1f5d7..243db12999 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -1951,12 +1951,13 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, } else { - int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length; + int len = std::min (ainfo->length, cinfo->xlen); + int align = std::max (ainfo->align, cinfo->xlen); if (!riscv_assign_reg_location (&ainfo->argloc[0], &cinfo->int_regs, len, 0)) riscv_assign_stack_location (&ainfo->argloc[0], - &cinfo->memory, len, ainfo->align); + &cinfo->memory, len, align); if (len < ainfo->length) { ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-08 15:43 ` Andrew Burgess @ 2018-11-13 1:52 ` Jim Wilson 0 siblings, 0 replies; 9+ messages in thread From: Jim Wilson @ 2018-11-13 1:52 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches On Thu, Nov 8, 2018 at 7:43 AM Andrew Burgess <andrew.burgess@embecosm.com> wrote: > I struggled to understand what the difference between align and > slot_align is in this patch, it feels like.... If you have an argument twice the size of xlen, with alignment twice the size of xlen, then when we pass the first word, align is 2*xlen and slot_align is xlen. > .... you might be better off just passing a better value of align > through here. Testing on gdb.base/gnu_vector.exp shows that doing > this fixes the same problem that your original patch fixes. > - int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length; > + int len = std::min (ainfo->length, cinfo->xlen); > + int align = std::max (ainfo->align, cinfo->xlen); If you do this, and we keep the patch in riscv_assign_stack_location, then we end up allocating 2*xlen bytes for the first xlen bytes of a argument of size 2*len whcih is wrong. I suppose you want to drop the riscv_assign_stack_location change too. That could work, but that leaves arg_offset unaligned after allocating the last argument. Though it looks like the only problem with that is some funny riscv_debug_infcall output. It might say for instance that we have 12 bytes of stack arguments for a 64-bit target, which doesn't make any sense. For a 64-bit target, stack arguments should always be a multiple of 8 bytes. Otherwise, this looks harmless, and this is a problem that we already have, so this isn't anything broken by this patch set. I'll rewrite the patch this way. Jim ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-06 21:43 [PATCH 0/3] RISC-V: gdb.base/gnu_vector fixes Jim Wilson 2018-11-06 21:44 ` [PATCH 3/3] " Jim Wilson 2018-11-06 21:44 ` [PATCH 1/3] " Jim Wilson @ 2018-11-06 21:44 ` Jim Wilson 2018-11-08 14:37 ` Andrew Burgess 2 siblings, 1 reply; 9+ messages in thread From: Jim Wilson @ 2018-11-06 21:44 UTC (permalink / raw) To: gdb-patches; +Cc: Jim Wilson GCC gives vectors natural aligment based on total size, not element size, bounded by the maximum supported type alignment. gdb/ * riscv-tdep.c (BIGGEST_ALIGNMENT): New. (riscv_type_alignment) <TYPE_CODE_ARRAY>: If TYPE_VECTOR, return min of TYPE_LENGTH and BIGGEST_ALIGNMENT. --- gdb/riscv-tdep.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index ac4f2533f4..3d4f7e3dcc 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -59,6 +59,9 @@ /* The stack must be 16-byte aligned. */ #define SP_ALIGNMENT 16 +/* The biggest alignment that the target supports. */ +#define BIGGEST_ALIGNMENT 16 + /* Forward declarations. */ static bool riscv_has_feature (struct gdbarch *gdbarch, char feature); @@ -1640,6 +1643,10 @@ riscv_type_alignment (struct type *t) return TYPE_LENGTH (t); case TYPE_CODE_ARRAY: + if (TYPE_VECTOR (t)) + return std::min (TYPE_LENGTH (t), (unsigned)BIGGEST_ALIGNMENT); + /* FALLTHROUGH */ + case TYPE_CODE_COMPLEX: return riscv_type_alignment (TYPE_TARGET_TYPE (t)); -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] RISC-V: gdb.base/gnu_vector fixes. 2018-11-06 21:44 ` [PATCH 2/3] " Jim Wilson @ 2018-11-08 14:37 ` Andrew Burgess 0 siblings, 0 replies; 9+ messages in thread From: Andrew Burgess @ 2018-11-08 14:37 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches * Jim Wilson <jimw@sifive.com> [2018-11-06 13:44:28 -0800]: > GCC gives vectors natural aligment based on total size, not element size, > bounded by the maximum supported type alignment. > > gdb/ > * riscv-tdep.c (BIGGEST_ALIGNMENT): New. > (riscv_type_alignment) <TYPE_CODE_ARRAY>: If TYPE_VECTOR, return min > of TYPE_LENGTH and BIGGEST_ALIGNMENT. As with patch #1, please update the patch title line. > --- > gdb/riscv-tdep.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index ac4f2533f4..3d4f7e3dcc 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -59,6 +59,9 @@ > /* The stack must be 16-byte aligned. */ > #define SP_ALIGNMENT 16 > > +/* The biggest alignment that the target supports. */ > +#define BIGGEST_ALIGNMENT 16 > + > /* Forward declarations. */ > static bool riscv_has_feature (struct gdbarch *gdbarch, char feature); > > @@ -1640,6 +1643,10 @@ riscv_type_alignment (struct type *t) > return TYPE_LENGTH (t); > > case TYPE_CODE_ARRAY: > + if (TYPE_VECTOR (t)) > + return std::min (TYPE_LENGTH (t), (unsigned)BIGGEST_ALIGNMENT); I think there's supposed to be a space after the cast, '(unsigned) BIGGEST_ALIGNMENT'. > + /* FALLTHROUGH */ > + > case TYPE_CODE_COMPLEX: > return riscv_type_alignment (TYPE_TARGET_TYPE (t)); > > -- > 2.17.1 > Otherwise, look fine. Thanks, Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-11-13 1:52 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-06 21:43 [PATCH 0/3] RISC-V: gdb.base/gnu_vector fixes Jim Wilson 2018-11-06 21:44 ` [PATCH 3/3] " Jim Wilson 2018-11-08 14:34 ` Andrew Burgess 2018-11-08 14:44 ` Andrew Burgess 2018-11-06 21:44 ` [PATCH 1/3] " Jim Wilson 2018-11-08 15:43 ` Andrew Burgess 2018-11-13 1:52 ` Jim Wilson 2018-11-06 21:44 ` [PATCH 2/3] " Jim Wilson 2018-11-08 14:37 ` Andrew Burgess
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).