This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [08/11] TI C6X binutils port: opcodes/


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]