[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 #5 from Hans-Peter Nilsson <hp at sourceware dot org> ---
(In reply to Mike Frysinger from comment #4)
> (In reply to Hans-Peter Nilsson from comment #3)
> > My point is that *these two files* would be the only files that got
> > -Wno-error=unused-variable.
> 
> i understand what you're suggesting.  my point still stands.  this assumes
> that the toolchain used to compile the sim recognizes & accepts (or silently
> ignores) "-Wno-error=unused-variable". 

I don't think that's necessary: warnings are only enabled for gcc, and for
developer mode.  Developers can be expected to have a sufficiently modern gcc. 
I see it works for gcc-10 (Debian 11).

> the sim, like all binutils+gdb
> projects, test the toolchain and whether it supports -W flags before using
> them.  we have to add a test for -Wno-error=unused-variable, a variable for
> it, and then expand that for these 2 files.

Nah, that seems a bit overthinking it.  Adding it when warnings are enabled
should be sufficient.  Alternatively, we can just add a "-Wno-error" like the
extant cases.

> seems a lot easier to fix the
> origin, especially when enabling warnings has a proven history of uncovering
> real bugs.
> 
> > You say, you've looked for, but not found, a
> > bug-reason why the warnings appear, in this generated code, presumably
> > including looking at the generator.
> 
> the generator appears to be cgen lisp which uses the lisp cpu definitions. 
> the cgen code is not easy to follow at all, or trace back which funcs
> generate which lines.  i was hoping you, as author of much of this code,
> would be able to pick out & fix things much quicker than i.

Fair enough.  I had a quick look.

First a look at the apparent sites of the warnings (N.B.: only the ones quoted
in this PR, I don't have a new enough gcc to emit those particular ones).  The
*immediate* causes for each and every one of those, i.e. why "insn" isn't used,
is that the insn in the self-named variable is fully decoded for the cases
corresponding to the warnings, and no sub-fields need to be inspected.  Any
operand is in the next 16- or 32-bit word.  The *immediate* cause is hereby
analyzed as innocuous and AFAIU typical for auto-generated code. ;)

CGEN apparently emits a generic template containing "CGEN_INSN_WORD insn =
base_insn;".  That template can be improved with a "generic" use of the
variable insn, like appending a "(void) insn;" in the generated code, with a
suitable comment nearby in the generator, like 'The insn may be fully decoded
at this point.  Add an artificial use to avoid compiler warnings about insn not
being used.'.

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.

(FAOD: the above is merely a suggestion; not volunteering myself.  I'd go with
the -Wno-error.)

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