[Bug sim/31181] sim: cris: decode unused base_insn variable warnings

["hp at sourceware dot org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=31181

--- Comment #9 from Hans-Peter Nilsson <hp at sourceware dot org> ---
(In reply to Mike Frysinger from comment #8)
> (In reply to Hans-Peter Nilsson from comment #5)
> > Developers can be expected to have a sufficiently modern gcc
> > ...
> > I don't have a new enough gcc to emit those particular ones
> 
> one person's modern is another's too-old ;)

Shades of grey and no absolutes...  New, but not new enough for the warnings
*you* see.  Some put gcc-10 in the "oldstable", soon enough "oldoldstable". 
When I compiled those files, I saw *other* warnings, some for uninitialized
variables, but probably false-positive-warning-bugs in that gcc version.

> > But, for that kind of change, this PR should be redirected to that project,
> > as the template is there, in cgen/sim-decode.scm according to git grep.
> 
> sure, if we think the right place is cgen, i can send it there.  i wasn't
> sure if it was something we'd do entirely there, or if we'd change
> cpu/cris.cpu, or both.
> 
> ideally there would be something in the cpu definition that cgen could key
> of off (like the arguments passed down, or what implicitly requested more
> fields be decoded, or something else).  if that's not feasible, then i'd go
> with:
> 
> --- a/sim-decode.scm
> +++ b/sim-decode.scm
> @@ -436,7 +436,14 @@ void
>     "    const IDESC *idesc = &" IDESC-TABLE-VAR "[itype];\n"
>     (if (> (length (sfmt-iflds sfmt)) 0)
>         (string-append
> -        "    CGEN_INSN_WORD insn = "
> +        "    "
> +        ; Some insns are fully decoded by the time they get here, so they
> won't
> +        ; access the insn variable.  Mark it as unused to avoid warnings.
> +        and don't need the i

That last line seems superfluous and should yield errors.  (Accidentally quoted
an untested diff?)

> +        (if (adata-integral-insn? CURRENT-ARCH)
> +            ""
> +            "ATTRIBUTE_UNUSED ")

Oh, they have "ATTRIBUTE_UNUSED".  Ok, that settles the question whether the
cgen project cares about unused variables: they do.  Beware that (IIRC) not all
compilers (old gcc, perhaps *cough* clang) support the same *location* for the
attribute (here, before the type).  The canonical position is after; "before
the ';'".  I see gcc-10 accepts it where you put it though, so I'm good.

> +        "CGEN_INSN_WORD insn = "
>          (if (adata-integral-insn? CURRENT-ARCH)
>              "entire_insn;\n"
>              "base_insn;\n"))

I don't know whether the "adata-integral-insn?" predicate has bearing on
whether insn can be "unused". Still: besides the superfluous line, I'd say time
to post it for review, with a link to this PR.

-- 
You are receiving this mail because:
You are on the CC list for the bug.