Discussion:
[PATCH 0/3 v2] RISC-V: gdb.base/gnu_vector fixes.
Jim Wilson
2018-11-13 05:32:15 UTC
Permalink
The second version of my patches to fix infcall related failures on a
rv64gc linux system, updated as per Andrew's review.

This was tested on a riscv64-linux system with the gdb testsuite, and
it fixes 2 failures without causing any regressions.

Jim
Jim Wilson
2018-11-13 05:37:50 UTC
Permalink
For riscv64-linux target, fixes
FAIL: gdb.base/gnu_vector.exp: call add_many_charvecs

Ensure that stack slots are always the same alignment as XLEN by rounding
up arg align to xlen.

gdb/
* riscv-tdep.c (riscv_call_arg_scalar_int): Use std::min when
setting len. New local align, set to max of arg align and xlen,
and pass to first riscv_assign_stack_location call.
---
gdb/riscv-tdep.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index db372e2163..f4650dfbf3 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1923,12 +1923,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)
{
--
2.17.1
Jim Wilson
2018-11-13 05:38:32 UTC
Permalink
For riscv64-linxu target, first half of fix for
FAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs

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 f4650dfbf3..58dca3b86e 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
Jim Wilson
2018-11-13 05:39:01 UTC
Permalink
For riscv64-linux target, second half of fix for
FAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs

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 | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 58dca3b86e..6b2f977efd 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
@@ -1933,6 +1936,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
int len = std::min (ainfo->length, cinfo->xlen);
int align = std::max (ainfo->align, cinfo->xlen);

+ /* Unnamed arguments in registers that require 2*XLEN alignment are
+ passed in an aligned register pair. */
+ if (ainfo->is_unnamed && (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],
@@ -2213,7 +2222,9 @@ riscv_call_arg_struct (struct riscv_arg_info *ainfo,
selected from CINFO which holds information about what call argument
locations are available for use next. The TYPE is the type of the
argument being passed, this information is recorded into AINFO (along
- with some additional information derived from the type).
+ with some additional information derived from the type). IS_UNNAMED
+ is true if this is an unnamed (stdarg) argument, this info is also
+ recorded into AINFO.

After assigning a location to AINFO, CINFO will have been updated. */

@@ -2221,11 +2232,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))
@@ -2374,6 +2386,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;
@@ -2387,7 +2404,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);
@@ -2564,7 +2582,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
Andrew Burgess
2018-11-14 21:03:34 UTC
Permalink
Post by Jim Wilson
The second version of my patches to fix infcall related failures on a
rv64gc linux system, updated as per Andrew's review.
This was tested on a riscv64-linux system with the gdb testsuite, and
it fixes 2 failures without causing any regressions.
I'm happy with all 3 of these.

Thanks,
Andrew
Jim Wilson
2018-11-14 23:06:50 UTC
Permalink
On Wed, Nov 14, 2018 at 1:03 PM Andrew Burgess
Post by Andrew Burgess
I'm happy with all 3 of these.
Thanks. Committed.

Jim

Loading...