* Re: [PATCH] sim: riscv: fix build breakage with rvv changes
[not found] <20211028205408.2228904-1-vineetg@rivosinc.com>
@ 2021-10-29 2:59 ` Nelson Chu
2021-10-29 3:19 ` Vineet Gupta
0 siblings, 1 reply; 7+ messages in thread
From: Nelson Chu @ 2021-10-29 2:59 UTC (permalink / raw)
To: Vineet Gupta, gdb-patches; +Cc: Kito Cheng, Dylan Reid, Binutils
Also send this patch to gdb mailing since there should be the right
place for the issues of gdb and sim.
I'm going to move the rvv stuff from the integration branch back to
the mainline, so we should need this in the mainline later. But
neither am I the developer or the maintainer of gdb, so we need the
gdb experts' help.
Thanks
Nelson
On Fri, Oct 29, 2021 at 4:54 AM Vineet Gupta <vineetg@rivosinc.com> wrote:
>
> The vector changes on binutils-integration-branch missed updates
> to sim causing build failure when build sim/gdb.
>
> This patch is only for user/riscv/binutils-integration-branch
>
> Fixes: 144cceb058e "(RISC-V/rvv: Add rvv v0.10 instructions.)"
> Reported-by: Dylan Reid <dylan@rivosinc.com>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
> sim/riscv/ChangeLog-2021 | 4 ++++
> sim/riscv/sim-main.c | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/sim/riscv/ChangeLog-2021 b/sim/riscv/ChangeLog-2021
> index e9aa74490f12..420b1867913c 100644
> --- a/sim/riscv/ChangeLog-2021
> +++ b/sim/riscv/ChangeLog-2021
> @@ -1,3 +1,7 @@
> +2021-20-28 Vineet Gupta <vineetg@revosinc.com>
> +
> + * sim-main.c (step_once): Fix match_func call per gas changes.
> +
> 2021-07-01 Mike Frysinger <vapier@gentoo.org>
>
> * configure: Regenerate.
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 0faf9395ae52..9b4f7c6c5aad 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -956,6 +956,7 @@ void step_once (SIM_CPU *cpu)
> sim_cia pc = cpu->pc;
> const struct riscv_opcode *op;
> int xlen = RISCV_XLEN (cpu);
> + const char *error = NULL;
>
> if (TRACE_ANY_P (cpu))
> trace_prefix (sd, cpu, NULL_CIA, pc, TRACE_LINENUM_P (cpu),
> @@ -985,7 +986,7 @@ void step_once (SIM_CPU *cpu)
> for (; op->name; op++)
> {
> /* Does the opcode match? */
> - if (! op->match_func (op, iw))
> + if (! op->match_func (op, iw, 0, /* check_constraints */ &error))
> continue;
> /* Is this a pseudo-instruction and may we print it as such? */
> if (op->pinfo & INSN_ALIAS)
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sim: riscv: fix build breakage with rvv changes
2021-10-29 2:59 ` [PATCH] sim: riscv: fix build breakage with rvv changes Nelson Chu
@ 2021-10-29 3:19 ` Vineet Gupta
0 siblings, 0 replies; 7+ messages in thread
From: Vineet Gupta @ 2021-10-29 3:19 UTC (permalink / raw)
To: Nelson Chu, gdb-patches; +Cc: Kito Cheng, Dylan Reid, Binutils
On 10/28/21 7:59 PM, Nelson Chu wrote:
> Also send this patch to gdb mailing since there should be the right
> place for the issues of gdb and sim.
Ok will do.
> I'm going to move the rvv stuff from the integration branch back to
> the mainline, so we should need this in the mainline later. But
> neither am I the developer or the maintainer of gdb, so we need the
> gdb experts' help.
Technically this change is simply matching the function prototype that
got changed as part of that rvv update. Since this is not mainline I
think gdb guys might hand wave as not their problem. Anyhow I'll send it
over and see what comes back. I presume riscv binutils/gdb still work
out of unified tree and branches ? So it should build off of integration
branch too.
-Vineet
>
> Thanks
> Nelson
>
> On Fri, Oct 29, 2021 at 4:54 AM Vineet Gupta <vineetg@rivosinc.com> wrote:
>> The vector changes on binutils-integration-branch missed updates
>> to sim causing build failure when build sim/gdb.
>>
>> This patch is only for user/riscv/binutils-integration-branch
>>
>> Fixes: 144cceb058e "(RISC-V/rvv: Add rvv v0.10 instructions.)"
>> Reported-by: Dylan Reid <dylan@rivosinc.com>
>> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
>> ---
>> sim/riscv/ChangeLog-2021 | 4 ++++
>> sim/riscv/sim-main.c | 3 ++-
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/sim/riscv/ChangeLog-2021 b/sim/riscv/ChangeLog-2021
>> index e9aa74490f12..420b1867913c 100644
>> --- a/sim/riscv/ChangeLog-2021
>> +++ b/sim/riscv/ChangeLog-2021
>> @@ -1,3 +1,7 @@
>> +2021-20-28 Vineet Gupta <vineetg@revosinc.com>
>> +
>> + * sim-main.c (step_once): Fix match_func call per gas changes.
>> +
>> 2021-07-01 Mike Frysinger <vapier@gentoo.org>
>>
>> * configure: Regenerate.
>> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
>> index 0faf9395ae52..9b4f7c6c5aad 100644
>> --- a/sim/riscv/sim-main.c
>> +++ b/sim/riscv/sim-main.c
>> @@ -956,6 +956,7 @@ void step_once (SIM_CPU *cpu)
>> sim_cia pc = cpu->pc;
>> const struct riscv_opcode *op;
>> int xlen = RISCV_XLEN (cpu);
>> + const char *error = NULL;
>>
>> if (TRACE_ANY_P (cpu))
>> trace_prefix (sd, cpu, NULL_CIA, pc, TRACE_LINENUM_P (cpu),
>> @@ -985,7 +986,7 @@ void step_once (SIM_CPU *cpu)
>> for (; op->name; op++)
>> {
>> /* Does the opcode match? */
>> - if (! op->match_func (op, iw))
>> + if (! op->match_func (op, iw, 0, /* check_constraints */ &error))
>> continue;
>> /* Is this a pseudo-instruction and may we print it as such? */
>> if (op->pinfo & INSN_ALIAS)
>> --
>> 2.30.2
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sim: riscv: fix build breakage with rvv changes
2021-10-29 19:28 Vineet Gupta
@ 2021-11-10 0:01 ` Vineet Gupta via Gdb-patches
2021-11-10 0:02 ` Vineet Gupta
2021-11-10 9:40 ` Andrew Burgess via Gdb-patches
2 siblings, 0 replies; 7+ messages in thread
From: Vineet Gupta via Gdb-patches @ 2021-11-10 0:01 UTC (permalink / raw)
To: Vineet Gupta, gdb-patches, binutils; +Cc: Dylan Reid, Kito Cheng
ping !
On 10/29/21 12:28 PM, Vineet Gupta wrote:
> changes to gas for riscv vector extensions need to be propagated to sim
> otherwise gdb fails to build on users/riscv/binutils-integration-branch
>
> This patch currently applies to that branch.
>
> Fixes: 144cceb058e "(RISC-V/rvv: Add rvv v0.10 instructions.)"
> Reported-by: Dylan Reid <dylan@rivosinc.com>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
> sim/riscv/ChangeLog-2021 | 4 ++++
> sim/riscv/sim-main.c | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/sim/riscv/ChangeLog-2021 b/sim/riscv/ChangeLog-2021
> index e9aa74490f12..9ced6773bdd6 100644
> --- a/sim/riscv/ChangeLog-2021
> +++ b/sim/riscv/ChangeLog-2021
> @@ -1,3 +1,7 @@
> +2021-20-28 Vineet Gupta <vineetg@rivosinc.com>
> +
> + * sim-main.c (step_once): Fix match_func call per gas changes.
> +
> 2021-07-01 Mike Frysinger <vapier@gentoo.org>
>
> * configure: Regenerate.
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 0faf9395ae52..9b4f7c6c5aad 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -956,6 +956,7 @@ void step_once (SIM_CPU *cpu)
> sim_cia pc = cpu->pc;
> const struct riscv_opcode *op;
> int xlen = RISCV_XLEN (cpu);
> + const char *error = NULL;
>
> if (TRACE_ANY_P (cpu))
> trace_prefix (sd, cpu, NULL_CIA, pc, TRACE_LINENUM_P (cpu),
> @@ -985,7 +986,7 @@ void step_once (SIM_CPU *cpu)
> for (; op->name; op++)
> {
> /* Does the opcode match? */
> - if (! op->match_func (op, iw))
> + if (! op->match_func (op, iw, 0, /* check_constraints */ &error))
> continue;
> /* Is this a pseudo-instruction and may we print it as such? */
> if (op->pinfo & INSN_ALIAS)
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sim: riscv: fix build breakage with rvv changes
2021-10-29 19:28 Vineet Gupta
2021-11-10 0:01 ` Vineet Gupta via Gdb-patches
@ 2021-11-10 0:02 ` Vineet Gupta
2021-11-10 9:40 ` Andrew Burgess via Gdb-patches
2 siblings, 0 replies; 7+ messages in thread
From: Vineet Gupta @ 2021-11-10 0:02 UTC (permalink / raw)
To: Vineet Gupta, gdb-patches, binutils; +Cc: Dylan Reid, Kito Cheng
ping
On 10/29/21 12:28 PM, Vineet Gupta wrote:
> changes to gas for riscv vector extensions need to be propagated to sim
> otherwise gdb fails to build on users/riscv/binutils-integration-branch
>
> This patch currently applies to that branch.
>
> Fixes: 144cceb058e "(RISC-V/rvv: Add rvv v0.10 instructions.)"
> Reported-by: Dylan Reid <dylan@rivosinc.com>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
> sim/riscv/ChangeLog-2021 | 4 ++++
> sim/riscv/sim-main.c | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/sim/riscv/ChangeLog-2021 b/sim/riscv/ChangeLog-2021
> index e9aa74490f12..9ced6773bdd6 100644
> --- a/sim/riscv/ChangeLog-2021
> +++ b/sim/riscv/ChangeLog-2021
> @@ -1,3 +1,7 @@
> +2021-20-28 Vineet Gupta <vineetg@rivosinc.com>
> +
> + * sim-main.c (step_once): Fix match_func call per gas changes.
> +
> 2021-07-01 Mike Frysinger <vapier@gentoo.org>
>
> * configure: Regenerate.
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 0faf9395ae52..9b4f7c6c5aad 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -956,6 +956,7 @@ void step_once (SIM_CPU *cpu)
> sim_cia pc = cpu->pc;
> const struct riscv_opcode *op;
> int xlen = RISCV_XLEN (cpu);
> + const char *error = NULL;
>
> if (TRACE_ANY_P (cpu))
> trace_prefix (sd, cpu, NULL_CIA, pc, TRACE_LINENUM_P (cpu),
> @@ -985,7 +986,7 @@ void step_once (SIM_CPU *cpu)
> for (; op->name; op++)
> {
> /* Does the opcode match? */
> - if (! op->match_func (op, iw))
> + if (! op->match_func (op, iw, 0, /* check_constraints */ &error))
> continue;
> /* Is this a pseudo-instruction and may we print it as such? */
> if (op->pinfo & INSN_ALIAS)
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sim: riscv: fix build breakage with rvv changes
2021-10-29 19:28 Vineet Gupta
2021-11-10 0:01 ` Vineet Gupta via Gdb-patches
2021-11-10 0:02 ` Vineet Gupta
@ 2021-11-10 9:40 ` Andrew Burgess via Gdb-patches
2021-11-10 17:12 ` Vineet Gupta
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess via Gdb-patches @ 2021-11-10 9:40 UTC (permalink / raw)
To: Vineet Gupta; +Cc: Dylan Reid, binutils, Kito Cheng, gdb-patches, Jim Wilson
* Vineet Gupta <vineetg@rivosinc.com> [2021-10-29 12:28:56 -0700]:
> changes to gas for riscv vector extensions need to be propagated to sim
> otherwise gdb fails to build on users/riscv/binutils-integration-branch
>
> This patch currently applies to that branch.
>
> Fixes: 144cceb058e "(RISC-V/rvv: Add rvv v0.10 instructions.)"
> Reported-by: Dylan Reid <dylan@rivosinc.com>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
> sim/riscv/ChangeLog-2021 | 4 ++++
> sim/riscv/sim-main.c | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/sim/riscv/ChangeLog-2021 b/sim/riscv/ChangeLog-2021
> index e9aa74490f12..9ced6773bdd6 100644
> --- a/sim/riscv/ChangeLog-2021
> +++ b/sim/riscv/ChangeLog-2021
> @@ -1,3 +1,7 @@
> +2021-20-28 Vineet Gupta <vineetg@rivosinc.com>
> +
> + * sim-main.c (step_once): Fix match_func call per gas changes.
> +
> 2021-07-01 Mike Frysinger <vapier@gentoo.org>
>
> * configure: Regenerate.
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 0faf9395ae52..9b4f7c6c5aad 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -956,6 +956,7 @@ void step_once (SIM_CPU *cpu)
> sim_cia pc = cpu->pc;
> const struct riscv_opcode *op;
> int xlen = RISCV_XLEN (cpu);
> + const char *error = NULL;
Could this not be moved to the more inner scope?
>
> if (TRACE_ANY_P (cpu))
> trace_prefix (sd, cpu, NULL_CIA, pc, TRACE_LINENUM_P (cpu),
> @@ -985,7 +986,7 @@ void step_once (SIM_CPU *cpu)
> for (; op->name; op++)
> {
> /* Does the opcode match? */
> - if (! op->match_func (op, iw))
> + if (! op->match_func (op, iw, 0, /* check_constraints */ &error))
> continue;
I've not looked at exactly what the purpose of error is here, does it
just provide a reason why this function returns false? i.e. is it
always OK for us to ignore it like this? Maybe a comment explaining
briefly why we ignore something called error would be helpful.
Maybe this email wasn't really intended for me, but you only need
approval from the branch owner before merging to a user branch, and
that certainly isn't me in this case, so I can't approve this patch
for the branch. I've added Nelson and Jim to the CC list as, along
with Kito, they wrote the original patch.
And, as this patch doesn't currently apply to master, I can't approve
this patch for master either.
I assume this fix will be merged into the original patch (commit
144cceb058e59977f in the user branch) before the work is officially
posted for inclusion in upstream master.
Thanks,
Andrew
> /* Is this a pseudo-instruction and may we print it as such? */
> if (op->pinfo & INSN_ALIAS)
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sim: riscv: fix build breakage with rvv changes
2021-11-10 9:40 ` Andrew Burgess via Gdb-patches
@ 2021-11-10 17:12 ` Vineet Gupta
0 siblings, 0 replies; 7+ messages in thread
From: Vineet Gupta @ 2021-11-10 17:12 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Dylan Reid, binutils, Kito Cheng, gdb-patches, Jim Wilson
Hi Andrew,
On 11/10/21 1:40 AM, Andrew Burgess wrote:
> * Vineet Gupta <vineetg@rivosinc.com> [2021-10-29 12:28:56 -0700]:
>
>> changes to gas for riscv vector extensions need to be propagated to sim
>> otherwise gdb fails to build on users/riscv/binutils-integration-branch
>>
>> This patch currently applies to that branch.
>>
>> Fixes: 144cceb058e "(RISC-V/rvv: Add rvv v0.10 instructions.)"
>> Reported-by: Dylan Reid <dylan@rivosinc.com>
>> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
>> ---
>> sim/riscv/ChangeLog-2021 | 4 ++++
>> sim/riscv/sim-main.c | 3 ++-
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/sim/riscv/ChangeLog-2021 b/sim/riscv/ChangeLog-2021
>> index e9aa74490f12..9ced6773bdd6 100644
>> --- a/sim/riscv/ChangeLog-2021
>> +++ b/sim/riscv/ChangeLog-2021
>> @@ -1,3 +1,7 @@
>> +2021-20-28 Vineet Gupta <vineetg@rivosinc.com>
>> +
>> + * sim-main.c (step_once): Fix match_func call per gas changes.
>> +
>> 2021-07-01 Mike Frysinger <vapier@gentoo.org>
>>
>> * configure: Regenerate.
>> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
>> index 0faf9395ae52..9b4f7c6c5aad 100644
>> --- a/sim/riscv/sim-main.c
>> +++ b/sim/riscv/sim-main.c
>> @@ -956,6 +956,7 @@ void step_once (SIM_CPU *cpu)
>> sim_cia pc = cpu->pc;
>> const struct riscv_opcode *op;
>> int xlen = RISCV_XLEN (cpu);
>> + const char *error = NULL;
> Could this not be moved to the more inner scope?
Yep.
>
>>
>> if (TRACE_ANY_P (cpu))
>> trace_prefix (sd, cpu, NULL_CIA, pc, TRACE_LINENUM_P (cpu),
>> @@ -985,7 +986,7 @@ void step_once (SIM_CPU *cpu)
>> for (; op->name; op++)
>> {
>> /* Does the opcode match? */
>> - if (! op->match_func (op, iw))
>> + if (! op->match_func (op, iw, 0, /* check_constraints */ &error))
>> continue;
> I've not looked at exactly what the purpose of error is here, does it
> just provide a reason why this function returns false? i.e. is it
> always OK for us to ignore it like this? Maybe a comment explaining
> briefly why we ignore something called error would be helpful.
op->match_op is shared between gas and sim. It was updated as part of
some gas change (in the "Fixes" tag) but missed the corresponding change
in sim - all I'm doing here is propagate that - w/o needing any extra
semantics needed for gas change.
> Maybe this email wasn't really intended for me, but you only need
> approval from the branch owner before merging to a user branch, and
> that certainly isn't me in this case, so I can't approve this patch
> for the branch.
I thought so too :-) and mentioned this to Nelson at the time of
submission on binutils mailing list.
> I've added Nelson and Jim to the CC list as, along
> with Kito, they wrote the original patch.
>
> And, as this patch doesn't currently apply to master, I can't approve
> this patch for master either.
>
> I assume this fix will be merged into the original patch (commit
> 144cceb058e59977f in the user branch) before the work is officially
> posted for inclusion in upstream master.
Yeah indeed that was the idea.
Thx for taking a look.
-Vineet
^ permalink raw reply [flat|nested] 7+ messages in thread