[08/11] TI C6X binutils port: opcodes/
DJ Delorie
dj@redhat.com
Wed Mar 24 22:34:00 GMT 2010
> they are just repeating the plain semantics of the code. If it is clear
> that a condition should not occur - say, in the code
>
> field = tic6x_field_from_fmt (fmt, enc->field_id);
> if (!field)
> abort ();
>
> that looks up a field explicitly mentioned in an opcode's description, and
> expects it to exist - then I don't think a comment is useful.
This case was the one that made me think of it, though. You look for
a CREG field, and if you find it, you blindly look for a Z field too -
and abort if it doesn't exist. That isn't a case where "it should be
there because we just checked for it", it's a case of telling a future
developer that "all insns that have a CREG are expected to have a Z
too." Of course, that's one of the ones you commented, so we agree :-)
> Furthermore, sometimes the abort serves the purpose of telling both the
> reader and the compiler that a particular condition cannot occur, and so
Except that gas #defines it to something else, not that it matters...
> I've made this change, introducing a separate operands_text array to
> track whether the textual value of a particular operand has yet been
> found (previously whether operands[op_num] was non-NULL was used for
> this). All
You could check for operands[n][0] being NUL too, but that can be
changed later.
Patch OK.
More information about the Binutils
mailing list