Instruction packing with --gstabs for d10v assembler

Nick Clifton nickc@redhat.com
Mon Nov 6 16:55:00 GMT 2000


Hi Matt,

: 2000-11-06  Matthew Hiller  <hiller@redhat.com>
: 
: 	* config/tc-d10v.c (flag_no_gstabs_packing): New variable.
: 	(md_longopts): New option --gstabs-no-packing.
: 	(md_show_usage): New option --gstabs-no-packing.
: 	(md_parse_option): New option --gstabs-no-packing.
: 	(d10v_cleanup): Writes pending instruction only if
: 	!outputting_line_debug_p || flag_no_gstabs_packing.

Sorry - not approved.

The patch is actually OK, but it needs some tidying up to be really
good:

  * Remember to add documentation for the new command line switch.  ie
    you need to include a patch to gas/doc/c-d10v.texi describing the
    new switch.

  * Strictly speaking you ought to provide an inverse command line
    option as well (ie --gstabs-packing), so that the default
    behavior can be restored if necessary.


      !  	 command line */

  * Please end your comments with a period followed by two spaces and
    then the closing comment characters.

  * Generally speaking it is easier to understand booleans with a
    positive sense, rather than a negative sense.  Thus I would have
    created a variable called 'allow_gstabs_packing' which the
    --no-gstabs-packing command line switch would have set to false.


     !       /* If cleanup was invoked because the assembler encountered,
     !  	 i.e., a user label, we write out the pending instruction. If

  * The first sentence of this comment needs some cleaning up.
    Perhaps you meant 'e.g.' instead of 'i.e.' ?

  * The control flow in the d10v_cleanup function could be tidied up.
    (This is optional).  Rather than having two if statements
    enclosing the only real piece of code, you could have one, reverse
    the logic, and insert a return statement.  Something like this: 

      int
      d10v_cleanup ()
      {
        segT seg;
        subsegT subseg;
      
        /* There is nothing to do if there is no pending instruction.
           Also, if cleanup was invoked because the assembler is
           outputting a piece of line debugging information then do
           not emit the instruction if the --no-gstabs-packing command
           line switch has been specified.  */
        if (! prev_opcode
            || etype != PACK_UNSPEC
            || (outputting_line_debug_p && ! allow_gstabs_packing))
          return;
      
        seg = now_seg;
        subseg = now_subseg;
      
        if (prev_seg)
          subseg_set (prev_seg, prev_subseg);
      
        write_1_short (prev_opcode, prev_insn, fixups->next);
        subseg_set (seg, subseg);
      
        prev_opcode = NULL;
      
        return 1;
      }

Cheers
	Nick


More information about the Binutils mailing list