This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: Instruction packing with --gstabs for d10v assembler
- To: hiller at redhat dot com
- Subject: Re: Instruction packing with --gstabs for d10v assembler
- From: Nick Clifton <nickc at redhat dot com>
- Date: Mon, 6 Nov 2000 16:55:11 -0800
- CC: binutils at sources dot redhat dot com
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