Re: [PATCH] sim: riscv: fix build breakage with rvv changes

[Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>]

* 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
>