public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: Patch to change ARM register name set
  1999-07-01  0:00 Patch to change ARM register name set Nick Clifton
@ 1999-07-01  0:00 ` Richard Earnshaw
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Earnshaw @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Nick Clifton; +Cc: richard.earnshaw

nickc@cygnus.com said:
> I like it.  OK here is a second revision of the patch, which
> implements your suggestions above: 

Err, the documentation hasn't been updated to reflect this.

R.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
@ 1999-07-01  0:00 Nick Clifton
  1999-07-01  0:00 ` Richard Earnshaw
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Clifton @ 1999-07-01  0:00 UTC (permalink / raw)
  To: binutils

Hi Richard,

: A couple of niggles rather than anything consequential.
: 
: Despite what I said about APCS reg names, the usage of r13, r14 and r15 is 
: so tightly defined by the architecture that I think it is worth preserving 
: the apcs-style names (sp, lr & pc respectively) for these even if using 
: the standard names.  Use of 'fp' for r11 is more debatable since this 
: really is an apcs option.
: 
: Second, the option to select this is, I think, a little unobvious unless 
: you already know what it is all about (what does "-M standard-names" 
: mean?).  I'd prefer the option to be
: 	--disassembler-options=reg-names-std
: or
: 	--disassembler-options=reg-names-apcs
: 
: That way it is clear that the option refers to the register names and not 
: to some other feature of disassembly.
: 
: Maybe there should be a
: 	--disassembler-options=reg-names-raw
: as well, that only uses r0-r15 as the names.

I like it.  OK here is a second revision of the patch, which
implements your suggestions above:

Cheers
	Nick

--ChangeLog for include------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com>

	* dis-asm.h (arm_toggle_regnames): New prototype.
	(struct diassemble_info): New field: disassembler_options.

--ChangeLog for opcodes------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com> & Drew Mosley <dmoseley@cygnus.com>

	* arm-dis.c (arm_regnames): Turn into a pointer to a register
	name set.
	(arm_regnames_standard): New variable: Array of ARM register
	names according to ARM instruction set nomenclature. 
	(arm_regnames_apcs): New variable: Array of ARM register names
	according to ARM Procedure Call Standard.
	(arm_regnames_raw): New variable: Array of ARM register names
	using just 'r' and the register number.
	(arm_toggle_regnames): New function: Toggle the chosen register set
	naming scheme.
	(parse_disassembler_options): New function: Parse any target
	disassembler command line options.
	(print_insn_big_arm): Call parse_disassembler_options if any
	are defined.
	(print_insn_little_arm): Call parse_disassembler_options if any
	are defined.

--ChangeLog for binutils------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com>

	* objdump.c (disassembler_options): New variable.
	(usage): Document new -M/--disassembler-options option.
	(long_options): Add --disassembler-options.
	(disassemble_data): Initialise disassembler_options field of
	disassembler_info structure.
	(main): Add parsing of -M option.
	
	* binutils.texi: Document new command line switch to objdump.

	* NEWS: Describe new command line switch to objdump.


Index: include/dis-asm.h
===================================================================
RCS file: /cvs/binutils/binutils/include/dis-asm.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 dis-asm.h
*** dis-asm.h	1999/05/03 07:29:01	1.1.1.1
--- dis-asm.h	1999/06/15 15:16:11
*************** typedef struct disassemble_info {
*** 133,138 ****
--- 133,141 ----
  				   zero if unknown.  */
    bfd_vma target2;		/* Second target address for dref2 */
  
+   /* Command line options specific to the target disassembler.  */
+   char * disassembler_options;
+ 
  } disassemble_info;
  
  \f
*************** extern int print_insn_v850		PARAMS ((bfd
*** 180,185 ****
--- 183,190 ----
  extern int print_insn_tic30		PARAMS ((bfd_vma, disassemble_info*));
  extern int print_insn_vax		PARAMS ((bfd_vma, disassemble_info*));
  extern int print_insn_tic80		PARAMS ((bfd_vma, disassemble_info*));
+ 
+ extern int arm_toggle_regnames          PARAMS ((void));
  
  /* Fetch the disassembler for a given BFD, if that support is available.  */
  extern disassembler_ftype disassembler	PARAMS ((bfd *));

Index: opcodes/arm-dis.c
===================================================================
RCS file: /cvs/binutils/binutils/opcodes/arm-dis.c,v
retrieving revision 1.2
diff -p -r1.2 arm-dis.c
*** arm-dis.c	1999/06/04 07:14:10	1.2
--- arm-dis.c	1999/06/15 15:16:12
***************
*** 1,5 ****
  /* Instruction printing code for the ARM
!    Copyright (C) 1994, 95, 96, 97, 1998 Free Software Foundation, Inc. 
     Contributed by Richard Earnshaw (rwe@pegasus.esprit.ec.org)
     Modification by James G. Smith (jsmith@cygnus.co.uk)
  
--- 1,5 ----
  /* Instruction printing code for the ARM
!    Copyright (C) 1994, 95, 96, 97, 1998, 1999 Free Software Foundation, Inc. 
     Contributed by Richard Earnshaw (rwe@pegasus.esprit.ec.org)
     Modification by James G. Smith (jsmith@cygnus.co.uk)
  
*************** static char *arm_conditional[] =
*** 35,44 ****
  {"eq", "ne", "cs", "cc", "mi", "pl", "vs", "vc",
   "hi", "ls", "ge", "lt", "gt", "le", "", "nv"};
  
! static char *arm_regnames[] =
  {"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
!  "r8", "r9", "sl", "fp", "ip", "sp", "lr", "pc"};
  
  static char *arm_fp_const[] =
  {"0.0", "1.0", "2.0", "3.0", "4.0", "5.0", "0.5", "10.0"};
  
--- 35,55 ----
  {"eq", "ne", "cs", "cc", "mi", "pl", "vs", "vc",
   "hi", "ls", "ge", "lt", "gt", "le", "", "nv"};
  
! static char *arm_regnames_raw[] =
  {"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
!  "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"};
  
+ static char *arm_regnames_standard[] =
+ {"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+  "r8", "r9", "r10", "r11", "r12", "sp", "lr", "pc"};
+ 
+ static char *arm_regnames_apcs[] =
+ {"a1", "a2", "a3", "a4", "v1", "v2", "v3", "v4",
+  "v5", "v6", "sl", "fp", "ip", "sp", "lr", "pc"};
+ 
+ /* Choose which register name set to use.  */
+ static char **arm_regnames = arm_regnames_standard;
+ 
  static char *arm_fp_const[] =
  {"0.0", "1.0", "2.0", "3.0", "4.0", "5.0", "0.5", "10.0"};
  
*************** print_insn_thumb (pc, info, given)
*** 742,747 ****
--- 753,797 ----
    abort ();
  }
  
+ /* Select a different register name set.
+    Returns true if the name set selected is the APCS name set.  */
+ int
+ arm_toggle_regnames ()
+ {
+   if (arm_regnames == arm_regnames_standard)
+     arm_regnames = arm_regnames_apcs;
+   else
+     arm_regnames = arm_regnames_standard;
+ 
+   return arm_regnames == arm_regnames_apcs;
+ }
+ 
+ static void
+ parse_disassembler_options (options)
+      char * options;
+ {
+   if (options == NULL)
+     return;
+       
+   if (strncmp (options, "reg-names-", 10) == 0)
+     {
+       options += 10;
+       
+       if (strcmp (options, "std") == 0)
+ 	arm_regnames = arm_regnames_standard;
+       else if (strcmp (options, "apcs") == 0)
+ 	arm_regnames = arm_regnames_apcs;
+       else if (strcmp (options, "raw") == 0)
+ 	arm_regnames = arm_regnames_raw;
+       else
+ 	fprintf (stderr, "Unrecognised register name set: %s\n", options);
+     }
+   else
+     fprintf (stderr, "Unrecognised disassembler option: %s\n", options);
+   
+   return;
+ }
+ 
  /* NOTE: There are no checks in these routines that the relevant number of data bytes exist */
  
  int
*************** print_insn_big_arm (pc, info)
*** 756,761 ****
--- 806,819 ----
    elf_symbol_type    *es;
    int                is_thumb;
    
+   if (info->disassembler_options)
+     {
+       parse_disassembler_options (info->disassembler_options);
+       
+       /* To avoid repeated parsing of this option, we remove it here.  */
+       info->disassembler_options = NULL;
+     }
+   
    is_thumb = false;
    if (info->symbols != NULL)
      {
*************** print_insn_little_arm (pc, info)
*** 838,843 ****
--- 896,909 ----
    elf_symbol_type    *es;
    int                is_thumb;
  
+   if (info->disassembler_options)
+     {
+       parse_disassembler_options (info->disassembler_options);
+       
+       /* To avoid repeated parsing of this option, we remove it here.  */
+       info->disassembler_options = NULL;
+     }
+   
    is_thumb = false;
    if (info->symbols != NULL)
      {

Index: binutils/objdump.c
===================================================================
RCS file: /cvs/binutils/binutils/binutils/objdump.c,v
retrieving revision 1.4
diff -p -r1.4 objdump.c
*** objdump.c	1999/06/13 19:02:25	1.4
--- objdump.c	1999/06/15 15:16:12
*************** struct objdump_disasm_info {
*** 83,88 ****
--- 83,91 ----
  /* Architecture to disassemble for, or default if NULL.  */
  static char *machine = (char *) NULL;
  
+ /* Target specific options to the disassembler.  */
+ static char *disassembler_options = (char *) NULL;
+ 
  /* Endianness to disassemble for, or default if BFD_ENDIAN_UNKNOWN.  */
  static enum bfd_endian endian = BFD_ENDIAN_UNKNOWN;
  
*************** usage (stream, status)
*** 217,223 ****
       int status;
  {
    fprintf (stream, _("\
! Usage: %s [-ahifCdDprRtTxsSlw] [-b bfdname] [-m machine] [-j section-name]\n\
         [--archive-headers] [--target=bfdname] [--debugging] [--disassemble]\n\
         [--disassemble-all] [--disassemble-zeroes] [--file-headers]\n\
         [--section-headers] [--headers]\n\
--- 220,227 ----
       int status;
  {
    fprintf (stream, _("\
! Usage: %s [-ahifCdDprRtTxsSlw] [-b bfdname] [-m machine] \n\
!        [-j section-name] [-M disassembler-options]\n\
         [--archive-headers] [--target=bfdname] [--debugging] [--disassemble]\n\
         [--disassemble-all] [--disassemble-zeroes] [--file-headers]\n\
         [--section-headers] [--headers]\n\
*************** static struct option long_options[]=
*** 255,260 ****
--- 259,265 ----
    {"demangle", no_argument, &do_demangle, 1},
    {"disassemble", no_argument, NULL, 'd'},
    {"disassemble-all", no_argument, NULL, 'D'},
+   {"disassembler-options", required_argument, NULL, 'M'},
    {"disassemble-zeroes", no_argument, &disassemble_zeroes, 1},
    {"dynamic-reloc", no_argument, NULL, 'R'},
    {"dynamic-syms", no_argument, NULL, 'T'},
*************** disassemble_data (abfd)
*** 1564,1569 ****
--- 1569,1576 ----
    disasm_info.flavour = bfd_get_flavour (abfd);
    disasm_info.arch = bfd_get_arch (abfd);
    disasm_info.mach = bfd_get_mach (abfd);
+   disasm_info.disassembler_options = disassembler_options;
+   
    if (bfd_big_endian (abfd))
      disasm_info.display_endian = disasm_info.endian = BFD_ENDIAN_BIG;
    else if (bfd_little_endian (abfd))
*************** main (argc, argv)
*** 2694,2700 ****
    bfd_init ();
    set_default_bfd_target ();
  
!   while ((c = getopt_long (argc, argv, "pib:m:VCdDlfahrRtTxsSj:wE:",
  			   long_options, (int *) 0))
  	 != EOF)
      {
--- 2701,2707 ----
    bfd_init ();
    set_default_bfd_target ();
  
!   while ((c = getopt_long (argc, argv, "pib:m:M:VCdDlfahrRtTxsSj:wE:",
  			   long_options, (int *) 0))
  	 != EOF)
      {
*************** main (argc, argv)
*** 2706,2711 ****
--- 2713,2721 ----
  	  break;		/* we've been given a long option */
  	case 'm':
  	  machine = optarg;
+ 	  break;
+ 	case 'M':
+ 	  disassembler_options = optarg;
  	  break;
  	case 'j':
  	  only = optarg;

Index: binutils/binutils.texi
===================================================================
RCS file: /cvs/binutils/binutils/binutils/binutils.texi,v
retrieving revision 1.4
diff -p -r1.4 binutils.texi
*** binutils.texi	1999/06/14 01:30:17	1.4
--- binutils.texi	1999/06/15 15:16:13
*************** objdump [ -a | --archive-headers ] 
*** 1137,1142 ****
--- 1137,1143 ----
          [ -j @var{section} | --section=@var{section} ]
          [ -l | --line-numbers ] [ -S | --source ]
          [ -m @var{machine} | --architecture=@var{machine} ]
+         [ -M @var{options} | --disassembler-options=@var{options}]
          [ -p | --private-headers ]
          [ -r | --reloc ] [ -R | --dynamic-reloc ]
          [ -s | --full-contents ]  [ --stabs ]
*************** Specify the architecture to use when dis
*** 1294,1299 ****
--- 1295,1312 ----
  can be useful when disassembling object files which do not describe
  architecture information, such as S-records.  You can list the available
  architectures with the @samp{-i} option.
+ 
+ @item -M @var{options}
+ @itemx --disassembler-options=@var{options}
+ Pass target specific information to the disassembler.  Only supported on
+ some targets.
+ 
+ If the target is an ARM architecture then this switch can be used to
+ select which register name set is used during disassembler.  Specifying
+ @samp{--disassembler-options=standard_names} (the default) will select the
+ register names as used in ARM's instruction set documentation.  Specifying
+ @samp{--disassembler-options=apcs_names} will select the name set used
+ by the ARM Procedure Call Standard. 
  
  @item -p
  @itemx --private-headers

Index: binutils/NEWS
===================================================================
RCS file: /cvs/binutils/binutils/binutils/NEWS,v
retrieving revision 1.3
diff -p -r1.3 NEWS
*** NEWS	1999/06/12 15:42:04	1.3
--- NEWS	1999/06/15 15:16:13
***************
*** 2,7 ****
--- 2,12 ----
  
  Changes in binutils 2.10:
  
+ * New command line switch to objdump -M (or --disassembler-options) which takes
+   a parameter which can then be interpreted on a per-target basis by the
+   disassembler.  Used by ARM targets to select register name sets, ISA or APCS
+   verions.
+   
  * objdump support for -mi386:intel which causes disassembly to be displayed
    with intel syntax.
  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Patch to change ARM register name set
@ 1999-07-01  0:00 Nick Clifton
  1999-07-01  0:00 ` Richard Earnshaw
  1999-07-01  0:00 ` Ian Lance Taylor
  0 siblings, 2 replies; 12+ messages in thread
From: Nick Clifton @ 1999-07-01  0:00 UTC (permalink / raw)
  To: binutils

Hi Guys,

  Does anyone have any comments on the patch below ?  It changes the
  ARM disassembler so that by default it uses the APCS register naming
  scheme, rather than the ISA register naming scheme.  It also
  introduces a new command line switch to objdump: -M or --target-data,
  which takes an argument which can be processed in a target specific
  manner.  If the target is the ARM then the text of the -M switch is
  checked to see if it is a register name set selector
  ("standard_names" or "apcs_names") and if so it chooses the
  appropriate name set. 

Cheers
	Nick

--ChangeLog for opcodes------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com> & Drew Mosley <dmoseley@cygnus.com>

	* arm-dis.c (arm_regnames): Turn into a pointer to either
	arm_regnames_arm_standard or arm_regnames_arm_apcs.
	(arm_regnames_arm_standard): New variable: Array of ARM register
	names according to ARM instruction nomenclature.
	(arm_regnames_arm_apcs): New variable: Array of ARM register names
	according to ARM Procedure Call Standard.
	(ar_toggle_regnames): New function: Toggle the chosen register set
	naming scheme.

	* disassemble.c (disassembler2): New function: Allow target
	specific data to override selection of disassembler function.

--ChangeLog for include------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com>

	* dis-asm.h (arm_toggle_regnames): New prototype.
	(disassembler2): New prototype.

--ChangeLog for binutils------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com>

	* objdump.c (target_data): New variable.
	(usage): Add -M/--target-data option.
	(long_options): Add --target-data.
	(disassemble_data): Call diassembler2().
	(main): Add parsing of -M option.
	
	* binutils.texi: Document new command line switch to objdump.

	* NEWS: Describe new command line switch to objdump.

Index: opcodes/arm-dis.c
===================================================================
RCS file: /cvs/binutils/binutils/opcodes/arm-dis.c,v
retrieving revision 1.2
diff -p -r1.2 arm-dis.c
*** arm-dis.c	1999/06/04 07:14:10	1.2
--- arm-dis.c	1999/06/14 14:06:42
***************
*** 1,5 ****
  /* Instruction printing code for the ARM
!    Copyright (C) 1994, 95, 96, 97, 1998 Free Software Foundation, Inc. 
     Contributed by Richard Earnshaw (rwe@pegasus.esprit.ec.org)
     Modification by James G. Smith (jsmith@cygnus.co.uk)
  
--- 1,5 ----
  /* Instruction printing code for the ARM
!    Copyright (C) 1994, 95, 96, 97, 1998, 1999 Free Software Foundation, Inc. 
     Contributed by Richard Earnshaw (rwe@pegasus.esprit.ec.org)
     Modification by James G. Smith (jsmith@cygnus.co.uk)
  
*************** static char *arm_conditional[] =
*** 35,44 ****
  {"eq", "ne", "cs", "cc", "mi", "pl", "vs", "vc",
   "hi", "ls", "ge", "lt", "gt", "le", "", "nv"};
  
! static char *arm_regnames[] =
  {"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
!  "r8", "r9", "sl", "fp", "ip", "sp", "lr", "pc"};
  
  static char *arm_fp_const[] =
  {"0.0", "1.0", "2.0", "3.0", "4.0", "5.0", "0.5", "10.0"};
  
--- 35,51 ----
  {"eq", "ne", "cs", "cc", "mi", "pl", "vs", "vc",
   "hi", "ls", "ge", "lt", "gt", "le", "", "nv"};
  
! static char *arm_regnames_arm_standard[] =
  {"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
!  "r8", "r9", "r10", "r11", "r12", "r13", "r14", "pc"};
  
+ static char *arm_regnames_arm_apcs[] =
+ {"a1", "a2", "a3", "a4", "v1", "v2", "v3", "v4",
+  "v5", "v6", "sl", "fp", "ip", "sp", "lr", "pc"};
+ 
+ /* Choose which register name set to use.  */
+ static char **arm_regnames = arm_regnames_arm_apcs;
+ 
  static char *arm_fp_const[] =
  {"0.0", "1.0", "2.0", "3.0", "4.0", "5.0", "0.5", "10.0"};
  
*************** print_insn_little_arm (pc, info)
*** 888,891 ****
--- 895,911 ----
      }
  
    return status;
+ }
+ 
+ /* Select a different register name set.
+    Returns true if the name set selected is the APCS name set.  */
+ int
+ arm_toggle_regnames ()
+ {
+   if (arm_regnames == arm_regnames_arm_standard)
+     arm_regnames = arm_regnames_arm_apcs;
+   else
+     arm_regnames = arm_regnames_arm_standard;
+ 
+   return arm_regnames == arm_regnames_arm_apcs;
  }

Index: opcodes/disassemble.c
===================================================================
RCS file: /cvs/binutils/binutils/opcodes/disassemble.c,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 disassemble.c
*** disassemble.c	1999/05/03 07:28:59	1.1.1.1
--- disassemble.c	1999/06/14 14:06:55
*************** disassembler (abfd)
*** 246,248 ****
--- 246,286 ----
    return disassemble;
  }
  
+ 
+ void
+ disassembler2 (abfd, disassembler, target_data)
+      bfd * abfd;
+      disassembler_ftype * disassembler;
+      char * target_data;
+ {
+   enum bfd_architecture a = bfd_get_arch (abfd);
+ 
+   switch (a)
+     {
+ #ifdef ARCH_arm
+     case bfd_arch_arm:
+       if (target_data == NULL)
+ 	break;
+       
+       if (strcmp (target_data, "-s") == 0
+ 	  || strcmp (target_data, "standard_names") == 0)
+ 	{
+ 	  if (arm_toggle_regnames ())
+ 	    arm_toggle_regnames ();
+ 	}
+       else if (strcmp (target_data, "-a") == 0
+ 	       || strcmp (target_data, "apcs_names") == 0)
+ 	{
+ 	  if (! arm_toggle_regnames ())
+ 	    arm_toggle_regnames ();
+ 	}
+       else
+ 	fprintf (stderr, "Unrecognised target_data data: %s\n", target_data);
+       break;
+ #endif
+     default:
+       break;
+     }
+ 
+   return;
+ }

Index: include/dis-asm.h
===================================================================
RCS file: /cvs/binutils/binutils/include/dis-asm.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 dis-asm.h
*** dis-asm.h	1999/05/03 07:29:01	1.1.1.1
--- dis-asm.h	1999/06/14 14:07:05
*************** extern int print_insn_tic30		PARAMS ((bf
*** 181,188 ****
--- 181,196 ----
  extern int print_insn_vax		PARAMS ((bfd_vma, disassemble_info*));
  extern int print_insn_tic80		PARAMS ((bfd_vma, disassemble_info*));
  
+ extern int arm_toggle_regnames          PARAMS ((void));
+ 
  /* Fetch the disassembler for a given BFD, if that support is available.  */
  extern disassembler_ftype disassembler	PARAMS ((bfd *));
+ 
+ /* Extended version of disassembler() function.  Takes two extra parameters,
+    the disassembler currently selected, so that it can be overridden if necessary,
+    and a string which can be interpreted on a per-target basis.  Objdump uses this
+    to pass the contents of the -M command line option (if given).  */
+ extern void disassembler2		PARAMS ((bfd *, disassembler_ftype *, char *));
  
  \f
  /* This block of definitions is for particular callers who read instructions

Index: binutils/objdump.c
===================================================================
RCS file: /cvs/binutils/binutils/binutils/objdump.c,v
retrieving revision 1.4
diff -p -r1.4 objdump.c
*** objdump.c	1999/06/13 19:02:25	1.4
--- objdump.c	1999/06/14 14:07:18
*************** struct objdump_disasm_info {
*** 83,88 ****
--- 83,91 ----
  /* Architecture to disassemble for, or default if NULL.  */
  static char *machine = (char *) NULL;
  
+ /* Target specific command line data.  Interpreted via a call to disassembler2().  */
+ static char *target_data = (char *) NULL;
+ 
  /* Endianness to disassemble for, or default if BFD_ENDIAN_UNKNOWN.  */
  static enum bfd_endian endian = BFD_ENDIAN_UNKNOWN;
  
*************** usage (stream, status)
*** 217,223 ****
       int status;
  {
    fprintf (stream, _("\
! Usage: %s [-ahifCdDprRtTxsSlw] [-b bfdname] [-m machine] [-j section-name]\n\
         [--archive-headers] [--target=bfdname] [--debugging] [--disassemble]\n\
         [--disassemble-all] [--disassemble-zeroes] [--file-headers]\n\
         [--section-headers] [--headers]\n\
--- 220,227 ----
       int status;
  {
    fprintf (stream, _("\
! Usage: %s [-ahifCdDprRtTxsSlw] [-b bfdname] [-m machine] \n\
!        [-j section-name] [-M target-data]\n\
         [--archive-headers] [--target=bfdname] [--debugging] [--disassemble]\n\
         [--disassemble-all] [--disassemble-zeroes] [--file-headers]\n\
         [--section-headers] [--headers]\n\
*************** static struct option long_options[]=
*** 277,282 ****
--- 281,287 ----
    {"stop-address", required_argument, NULL, OPTION_STOP_ADDRESS},
    {"syms", no_argument, NULL, 't'},
    {"target", required_argument, NULL, 'b'},
+   {"target-data", required_argument, NULL, 'M'},
    {"version", no_argument, &show_version, 1},
    {"wide", no_argument, &wide_output, 'w'},
    {0, no_argument, 0, 0}
*************** disassemble_data (abfd)
*** 1561,1566 ****
--- 1566,1575 ----
        return;
      }
  
+   /* Allow the target to override the selected disassembler based on
+      data provided by the command line switch -M.  */
+   disassembler2 (abfd, & disassemble_fn, target_data);
+   
    disasm_info.flavour = bfd_get_flavour (abfd);
    disasm_info.arch = bfd_get_arch (abfd);
    disasm_info.mach = bfd_get_mach (abfd);
*************** main (argc, argv)
*** 2694,2700 ****
    bfd_init ();
    set_default_bfd_target ();
  
!   while ((c = getopt_long (argc, argv, "pib:m:VCdDlfahrRtTxsSj:wE:",
  			   long_options, (int *) 0))
  	 != EOF)
      {
--- 2703,2709 ----
    bfd_init ();
    set_default_bfd_target ();
  
!   while ((c = getopt_long (argc, argv, "pib:m:M:VCdDlfahrRtTxsSj:wE:",
  			   long_options, (int *) 0))
  	 != EOF)
      {
*************** main (argc, argv)
*** 2706,2711 ****
--- 2715,2723 ----
  	  break;		/* we've been given a long option */
  	case 'm':
  	  machine = optarg;
+ 	  break;
+ 	case 'M':
+ 	  target_data = optarg;
  	  break;
  	case 'j':
  	  only = optarg;

Index: binutils/binutils.texi
===================================================================
RCS file: /cvs/binutils/binutils/binutils/binutils.texi,v
retrieving revision 1.4
diff -p -r1.4 binutils.texi
*** binutils.texi	1999/06/14 01:30:17	1.4
--- binutils.texi	1999/06/14 14:07:31
*************** objdump [ -a | --archive-headers ] 
*** 1137,1142 ****
--- 1137,1143 ----
          [ -j @var{section} | --section=@var{section} ]
          [ -l | --line-numbers ] [ -S | --source ]
          [ -m @var{machine} | --architecture=@var{machine} ]
+         [ -M @var{target_data} | --target_data=@var{target_data}]
          [ -p | --private-headers ]
          [ -r | --reloc ] [ -R | --dynamic-reloc ]
          [ -s | --full-contents ]  [ --stabs ]
*************** Specify the architecture to use when dis
*** 1294,1299 ****
--- 1295,1312 ----
  can be useful when disassembling object files which do not describe
  architecture information, such as S-records.  You can list the available
  architectures with the @samp{-i} option.
+ 
+ @item -M @var{target_data}
+ @itemx --target-data=@var{target_data}
+ Pass target specific information tot he disassembler.  Only support on
+ some targets.
+ 
+ If the target is an ARM architecture then this switch can be used to
+ select which register name set is used during disassembler.  Specifying
+ @samp{--target-data=standard_names} will select the register names as
+ used in ARM's instruction set documentation.  Specifying
+ @samp{--target-data=apcs_names} (the default) will select the name set
+ used by the ARM Procedure Call Standard.
  
  @item -p
  @itemx --private-headers

Index: binutils/NEWS
===================================================================
RCS file: /cvs/binutils/binutils/binutils/NEWS,v
retrieving revision 1.3
diff -p -r1.3 NEWS
*** NEWS	1999/06/12 15:42:04	1.3
--- NEWS	1999/06/14 14:07:43
***************
*** 2,7 ****
--- 2,12 ----
  
  Changes in binutils 2.10:
  
+ * New command line switch to objdump -M (or --target-data) which takes a
+   parameter which is then interpreted on a per-target basis.  Used by ARM
+   targets to select register name sets (standard_names or apcs_names) when
+   displaying disassembly.
+   
  * objdump support for -mi386:intel which causes disassembly to be displayed
    with intel syntax.
  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
  1999-07-01  0:00 Nick Clifton
@ 1999-07-01  0:00 ` Richard Earnshaw
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Earnshaw @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Nick Clifton; +Cc: richard.earnshaw

A couple of niggles rather than anything consequential.

Despite what I said about APCS reg names, the usage of r13, r14 and r15 is 
so tightly defined by the architecture that I think it is worth preserving 
the apcs-style names (sp, lr & pc respectively) for these even if using 
the standard names.  Use of 'fp' for r11 is more debatable since this 
really is an apcs option.

Second, the option to select this is, I think, a little unobvious unless 
you already know what it is all about (what does "-M standard-names" 
mean?).  I'd prefer the option to be
	--disassembler-options=reg-names-std
or
	--disassembler-options=reg-names-apcs

That way it is clear that the option refers to the register names and not 
to some other feature of disassembly.

Maybe there should be a
	--disassembler-options=reg-names-raw
as well, that only uses r0-r15 as the names.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
@ 1999-07-01  0:00 Nick Clifton
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Clifton @ 1999-07-01  0:00 UTC (permalink / raw)
  To: binutils

Hi Richard,

: > I like it.  OK here is a second revision of the patch, which
: > implements your suggestions above: 
: 
: Err, the documentation hasn't been updated to reflect this.

oops - assume that the patch ends like this:

Cheers
	Nick

Index: binutils/binutils.texi
===================================================================
RCS file: /cvs/binutils/binutils/binutils/binutils.texi,v
retrieving revision 1.4
diff -p -r1.4 binutils.texi
*** binutils.texi	1999/06/14 01:30:17	1.4
--- binutils.texi	1999/06/15 15:48:44
*************** objdump [ -a | --archive-headers ] 
*** 1137,1142 ****
--- 1137,1143 ----
          [ -j @var{section} | --section=@var{section} ]
          [ -l | --line-numbers ] [ -S | --source ]
          [ -m @var{machine} | --architecture=@var{machine} ]
+         [ -M @var{options} | --disassembler-options=@var{options}]
          [ -p | --private-headers ]
          [ -r | --reloc ] [ -R | --dynamic-reloc ]
          [ -s | --full-contents ]  [ --stabs ]
*************** Specify the architecture to use when dis
*** 1294,1299 ****
--- 1295,1315 ----
  can be useful when disassembling object files which do not describe
  architecture information, such as S-records.  You can list the available
  architectures with the @samp{-i} option.
+ 
+ @item -M @var{options}
+ @itemx --disassembler-options=@var{options}
+ Pass target specific information to the disassembler.  Only supported on
+ some targets.
+ 
+ If the target is an ARM architecture then this switch can be used to
+ select which register name set is used during disassembler.  Specifying
+ @samp{--disassembler-options=reg-name-std} (the default) will select the
+ register names as used in ARM's instruction set documentation, but with
+ register 13 called 'sp', register 14 called 'lr' and register 15 called
+ 'pc'.  Specifying @samp{--disassembler-options=reg-names-apcs} will
+ select the name set used by the ARM Procedure Call Standard, whilst
+ specifying @samp{--disassembler-options=reg-names-raw} will just use
+ @samp{r} followed by the register number.
  
  @item -p
  @itemx --private-headers

Index: binutils/NEWS
===================================================================
RCS file: /cvs/binutils/binutils/binutils/NEWS,v
retrieving revision 1.3
diff -p -r1.3 NEWS
*** NEWS	1999/06/12 15:42:04	1.3
--- NEWS	1999/06/15 15:48:44
***************
*** 2,7 ****
--- 2,12 ----
  
  Changes in binutils 2.10:
  
+ * New command line switch to objdump -M (or --disassembler-options) which takes
+   a parameter which can then be interpreted on a per-target basis by the
+   disassembler.  Used by ARM targets to select register name sets, ISA, APCS or
+   raw verions.
+   
  * objdump support for -mi386:intel which causes disassembly to be displayed
    with intel syntax.
  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
  1999-07-01  0:00 Nick Clifton
@ 1999-07-01  0:00 ` Scott Bambrough
  0 siblings, 0 replies; 12+ messages in thread
From: Scott Bambrough @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Nick Clifton, binutils

Nick Clifton wrote:
> 
> Hi Richard,
> 
> : Personally, I can't stand the APCS register names.  They seem
> : illogical to me.
> 
> I agree with you 100%.

Me too!

> No - it really needs to be a run time option.  However I am quite
> happy to change it so that the standard names are the default and the
> APCS names are only selected via a command line option.

I don't really want the APCS names as default.  I'd appreciate this change. 

Scott
begin:          vcard
fn:             Scott Bambrough
n:              Bambrough;Scott
org:            Rebel.com
adr:            150 Isabella St.;;;Ottawa;Ontario;K1S 1V7;Canada
email;internet: scottb@netwinder.org
title:          Software Engineer
tel;work:       (613) 788-6000 x6116
tel;fax:        (613) 230-8300
x-mozilla-cpt:  ;0
x-mozilla-html: FALSE
version:        2.1
end:            vcard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
@ 1999-07-01  0:00 Nick Clifton
  1999-07-01  0:00 ` Richard Earnshaw
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Clifton @ 1999-07-01  0:00 UTC (permalink / raw)
  To: binutils

Hi Guys,

  Here is a revised patch for you to look at.  This version:

  * Keeps the default ARM register name set as the ISA names.

  * Stores the text of the command line option in a new field in the
    disassembler_info structure rather than creating a new API
    function.

  * Puts the register name selection code entirely in arm-dis.c.

  * Changes the long version of the command line option used by
    objdump to '--disassembler-options=....'.

Any more comments ?

Cheers
	Nick

--ChangeLog for include------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com>

	* dis-asm.h (arm_toggle_regnames): New prototype.
	(struct diassemble_info): New field: disassembler_options.

--ChangeLog for opcodes------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com> & Drew Mosley <dmoseley@cygnus.com>

	* arm-dis.c (arm_regnames): Turn into a pointer to either
	arm_regnames_arm_standard or arm_regnames_arm_apcs.
	(arm_regnames_arm_standard): New variable: Array of ARM register
	names according to ARM instruction nomenclature.
	(arm_regnames_arm_apcs): New variable: Array of ARM register names
	according to ARM Procedure Call Standard.
	(arm_toggle_regnames): New function: Toggle the chosen register set
	naming scheme.
	(parse_disassembler_options): New function: Parse any target
	disassembler command line options.
	(print_insn_big_arm): Call parse_disassembler_options if any
	are defined.
	(print_insn_little_arm): Call parse_disassembler_options if any
	are defined.

--ChangeLog for binutils------------------------------------------------

1999-06-14  Nick Clifton  <nickc@cygnus.com>

	* objdump.c (disassembler_options): New variable.
	(usage): Document new -M/--disassembler-options option.
	(long_options): Add --disassembler-options.
	(disassemble_data): Initialise disassembler_options field of
	disassembler_info structure.
	(main): Add parsing of -M option.
	
	* binutils.texi: Document new command line switch to objdump.

	* NEWS: Describe new command line switch to objdump.

Index: include/dis-asm.h
===================================================================
RCS file: /cvs/binutils/binutils/include/dis-asm.h,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 dis-asm.h
*** dis-asm.h	1999/05/03 07:29:01	1.1.1.1
--- dis-asm.h	1999/06/15 12:42:51
*************** typedef struct disassemble_info {
*** 133,138 ****
--- 133,141 ----
  				   zero if unknown.  */
    bfd_vma target2;		/* Second target address for dref2 */
  
+   /* Command line options specific to the target disassembler.  */
+   char * disassembler_options;
+ 
  } disassemble_info;
  
  \f
*************** extern int print_insn_v850		PARAMS ((bfd
*** 180,185 ****
--- 183,190 ----
  extern int print_insn_tic30		PARAMS ((bfd_vma, disassemble_info*));
  extern int print_insn_vax		PARAMS ((bfd_vma, disassemble_info*));
  extern int print_insn_tic80		PARAMS ((bfd_vma, disassemble_info*));
+ 
+ extern int arm_toggle_regnames          PARAMS ((void));
  
  /* Fetch the disassembler for a given BFD, if that support is available.  */
  extern disassembler_ftype disassembler	PARAMS ((bfd *));

Index: opcodes/arm-dis.c
===================================================================
RCS file: /cvs/binutils/binutils/opcodes/arm-dis.c,v
retrieving revision 1.2
diff -p -r1.2 arm-dis.c
*** arm-dis.c	1999/06/04 07:14:10	1.2
--- arm-dis.c	1999/06/15 12:43:08
***************
*** 1,5 ****
  /* Instruction printing code for the ARM
!    Copyright (C) 1994, 95, 96, 97, 1998 Free Software Foundation, Inc. 
     Contributed by Richard Earnshaw (rwe@pegasus.esprit.ec.org)
     Modification by James G. Smith (jsmith@cygnus.co.uk)
  
--- 1,5 ----
  /* Instruction printing code for the ARM
!    Copyright (C) 1994, 95, 96, 97, 1998, 1999 Free Software Foundation, Inc. 
     Contributed by Richard Earnshaw (rwe@pegasus.esprit.ec.org)
     Modification by James G. Smith (jsmith@cygnus.co.uk)
  
*************** static char *arm_conditional[] =
*** 35,44 ****
  {"eq", "ne", "cs", "cc", "mi", "pl", "vs", "vc",
   "hi", "ls", "ge", "lt", "gt", "le", "", "nv"};
  
! static char *arm_regnames[] =
  {"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
!  "r8", "r9", "sl", "fp", "ip", "sp", "lr", "pc"};
  
  static char *arm_fp_const[] =
  {"0.0", "1.0", "2.0", "3.0", "4.0", "5.0", "0.5", "10.0"};
  
--- 35,51 ----
  {"eq", "ne", "cs", "cc", "mi", "pl", "vs", "vc",
   "hi", "ls", "ge", "lt", "gt", "le", "", "nv"};
  
! static char *arm_regnames_arm_standard[] =
  {"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
!  "r8", "r9", "r10", "r11", "r12", "r13", "r14", "pc"};
  
+ static char *arm_regnames_arm_apcs[] =
+ {"a1", "a2", "a3", "a4", "v1", "v2", "v3", "v4",
+  "v5", "v6", "sl", "fp", "ip", "sp", "lr", "pc"};
+ 
+ /* Choose which register name set to use.  */
+ static char **arm_regnames = arm_regnames_arm_standard;
+ 
  static char *arm_fp_const[] =
  {"0.0", "1.0", "2.0", "3.0", "4.0", "5.0", "0.5", "10.0"};
  
*************** print_insn_thumb (pc, info, given)
*** 742,747 ****
--- 749,792 ----
    abort ();
  }
  
+ /* Select a different register name set.
+    Returns true if the name set selected is the APCS name set.  */
+ int
+ arm_toggle_regnames ()
+ {
+   if (arm_regnames == arm_regnames_arm_standard)
+     arm_regnames = arm_regnames_arm_apcs;
+   else
+     arm_regnames = arm_regnames_arm_standard;
+ 
+   return arm_regnames == arm_regnames_arm_apcs;
+ }
+ 
+ static void
+ parse_disassembler_options (options)
+      char * options;
+ {
+   if (options == NULL)
+     return;
+       
+   if (strcmp (options, "-s") == 0
+       || strcmp (options, "standard_names") == 0)
+     {
+       if (arm_toggle_regnames ())
+ 	arm_toggle_regnames ();
+     }
+   else if (strcmp (options, "-a") == 0
+ 	   || strcmp (options, "apcs_names") == 0)
+     {
+       if (! arm_toggle_regnames ())
+ 	arm_toggle_regnames ();
+     }
+   else
+     fprintf (stderr, "Unrecognised disassembler option: %s\n", options);
+   
+   return;
+ }
+ 
  /* NOTE: There are no checks in these routines that the relevant number of data bytes exist */
  
  int
*************** print_insn_big_arm (pc, info)
*** 756,761 ****
--- 801,814 ----
    elf_symbol_type    *es;
    int                is_thumb;
    
+   if (info->disassembler_options)
+     {
+       parse_disassembler_options (info->disassembler_options);
+       
+       /* To avoid repeated parsing of this option, we remove it here.  */
+       info->disassembler_options = NULL;
+     }
+   
    is_thumb = false;
    if (info->symbols != NULL)
      {
*************** print_insn_little_arm (pc, info)
*** 838,843 ****
--- 891,904 ----
    elf_symbol_type    *es;
    int                is_thumb;
  
+   if (info->disassembler_options)
+     {
+       parse_disassembler_options (info->disassembler_options);
+       
+       /* To avoid repeated parsing of this option, we remove it here.  */
+       info->disassembler_options = NULL;
+     }
+   
    is_thumb = false;
    if (info->symbols != NULL)
      {

Index: binutils/objdump.c
===================================================================
RCS file: /cvs/binutils/binutils/binutils/objdump.c,v
retrieving revision 1.4
diff -p -r1.4 objdump.c
*** objdump.c	1999/06/13 19:02:25	1.4
--- objdump.c	1999/06/15 12:43:32
*************** struct objdump_disasm_info {
*** 83,88 ****
--- 83,91 ----
  /* Architecture to disassemble for, or default if NULL.  */
  static char *machine = (char *) NULL;
  
+ /* Target specific options to the disassembler.  */
+ static char *disassembler_options = (char *) NULL;
+ 
  /* Endianness to disassemble for, or default if BFD_ENDIAN_UNKNOWN.  */
  static enum bfd_endian endian = BFD_ENDIAN_UNKNOWN;
  
*************** usage (stream, status)
*** 217,223 ****
       int status;
  {
    fprintf (stream, _("\
! Usage: %s [-ahifCdDprRtTxsSlw] [-b bfdname] [-m machine] [-j section-name]\n\
         [--archive-headers] [--target=bfdname] [--debugging] [--disassemble]\n\
         [--disassemble-all] [--disassemble-zeroes] [--file-headers]\n\
         [--section-headers] [--headers]\n\
--- 220,227 ----
       int status;
  {
    fprintf (stream, _("\
! Usage: %s [-ahifCdDprRtTxsSlw] [-b bfdname] [-m machine] \n\
!        [-j section-name] [-M disassembler-options]\n\
         [--archive-headers] [--target=bfdname] [--debugging] [--disassemble]\n\
         [--disassemble-all] [--disassemble-zeroes] [--file-headers]\n\
         [--section-headers] [--headers]\n\
*************** static struct option long_options[]=
*** 255,260 ****
--- 259,265 ----
    {"demangle", no_argument, &do_demangle, 1},
    {"disassemble", no_argument, NULL, 'd'},
    {"disassemble-all", no_argument, NULL, 'D'},
+   {"disassembler-options", required_argument, NULL, 'M'},
    {"disassemble-zeroes", no_argument, &disassemble_zeroes, 1},
    {"dynamic-reloc", no_argument, NULL, 'R'},
    {"dynamic-syms", no_argument, NULL, 'T'},
*************** disassemble_data (abfd)
*** 1564,1569 ****
--- 1569,1576 ----
    disasm_info.flavour = bfd_get_flavour (abfd);
    disasm_info.arch = bfd_get_arch (abfd);
    disasm_info.mach = bfd_get_mach (abfd);
+   disasm_info.disassembler_options = disassembler_options;
+   
    if (bfd_big_endian (abfd))
      disasm_info.display_endian = disasm_info.endian = BFD_ENDIAN_BIG;
    else if (bfd_little_endian (abfd))
*************** main (argc, argv)
*** 2694,2700 ****
    bfd_init ();
    set_default_bfd_target ();
  
!   while ((c = getopt_long (argc, argv, "pib:m:VCdDlfahrRtTxsSj:wE:",
  			   long_options, (int *) 0))
  	 != EOF)
      {
--- 2701,2707 ----
    bfd_init ();
    set_default_bfd_target ();
  
!   while ((c = getopt_long (argc, argv, "pib:m:M:VCdDlfahrRtTxsSj:wE:",
  			   long_options, (int *) 0))
  	 != EOF)
      {
*************** main (argc, argv)
*** 2706,2711 ****
--- 2713,2721 ----
  	  break;		/* we've been given a long option */
  	case 'm':
  	  machine = optarg;
+ 	  break;
+ 	case 'M':
+ 	  disassembler_options = optarg;
  	  break;
  	case 'j':
  	  only = optarg;

Index: binutils/binutils.texi
===================================================================
RCS file: /cvs/binutils/binutils/binutils/binutils.texi,v
retrieving revision 1.4
diff -p -r1.4 binutils.texi
*** binutils.texi	1999/06/14 01:30:17	1.4
--- binutils.texi	1999/06/15 12:43:44
*************** objdump [ -a | --archive-headers ] 
*** 1137,1142 ****
--- 1137,1143 ----
          [ -j @var{section} | --section=@var{section} ]
          [ -l | --line-numbers ] [ -S | --source ]
          [ -m @var{machine} | --architecture=@var{machine} ]
+         [ -M @var{options} | --disassembler-options=@var{options}]
          [ -p | --private-headers ]
          [ -r | --reloc ] [ -R | --dynamic-reloc ]
          [ -s | --full-contents ]  [ --stabs ]
*************** Specify the architecture to use when dis
*** 1294,1299 ****
--- 1295,1312 ----
  can be useful when disassembling object files which do not describe
  architecture information, such as S-records.  You can list the available
  architectures with the @samp{-i} option.
+ 
+ @item -M @var{options}
+ @itemx --disassembler-options=@var{options}
+ Pass target specific information to the disassembler.  Only supported on
+ some targets.
+ 
+ If the target is an ARM architecture then this switch can be used to
+ select which register name set is used during disassembler.  Specifying
+ @samp{--disassembler-options=standard_names} (the default) will select the
+ register names as used in ARM's instruction set documentation.  Specifying
+ @samp{--disassembler-options=apcs_names} will select the name set used
+ by the ARM Procedure Call Standard. 
  
  @item -p
  @itemx --private-headers

Index: binutils/NEWS
===================================================================
RCS file: /cvs/binutils/binutils/binutils/NEWS,v
retrieving revision 1.3
diff -p -r1.3 NEWS
*** NEWS	1999/06/12 15:42:04	1.3
--- NEWS	1999/06/15 12:43:54
***************
*** 2,7 ****
--- 2,12 ----
  
  Changes in binutils 2.10:
  
+ * New command line switch to objdump -M (or --disassembler-options) which takes
+   a parameter which can then be interpreted on a per-target basis by the
+   disassembler.  Used by ARM targets to select register name sets, ISA or APCS
+   verions.
+   
  * objdump support for -mi386:intel which causes disassembly to be displayed
    with intel syntax.
  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
  1999-07-01  0:00 Nick Clifton
@ 1999-07-01  0:00 ` Richard Earnshaw
  1999-07-01  0:00 ` Ian Lance Taylor
  1 sibling, 0 replies; 12+ messages in thread
From: Richard Earnshaw @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Nick Clifton; +Cc: richard.earnshaw

nickc@cygnus.com said:
>   Does anyone have any comments on the patch below ?  It changes the
>   ARM disassembler so that by default it uses the APCS register naming
>   scheme, rather than the ISA register naming scheme.  It also
>   introduces a new command line switch to objdump: -M or
> --target-data,
>   which takes an argument which can be processed in a target specific
>   manner.  If the target is the ARM then the text of the -M switch is
>   checked to see if it is a register name set selector
>   ("standard_names" or "apcs_names") and if so it chooses the
>   appropriate name set. 


Personally, I can't stand the APCS register names.  They seem illogical to 
me.  Added to this is the fact that certain APCS options are supposed to 
change the bindings of some names: for example r9 is sometimes known as 
either sb or v6 depending on the options supplied to the assembler; 
similarly r10 can be either sl or v7 (I think I even saw a situation once 
where r10 was called v6 because r9 was being used for sb, but r10 was not 
being used for sl).  In other words, it isn't even obvious if such names 
are static.

Thumb uses yet another set of aliases for some of these registers, due to 
the restrictions on accessing the high registers from many instruction 
formats.

Finally, I thought that there was already a command built into gdb that 
could change the register names to the apcs variant if desired; this can 
be put into an init script, so I see little point in the need for a 
command-line option to do this as well.

If we must add this, can't we make it part of the configuration options, 
then a target which uses the current convention doesn't change under 
people's feet.

Richard.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
  1999-07-01  0:00 Nick Clifton
@ 1999-07-01  0:00 ` Ian Lance Taylor
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Lance Taylor @ 1999-07-01  0:00 UTC (permalink / raw)
  To: nickc; +Cc: binutils

   Date: Tue, 15 Jun 1999 10:57:16 +0100
   From: Nick Clifton <nickc@cygnus.com>

   : However, adding a new disassembler2 function does not make sense to
   : me.  Adding new functions because you have a new parameter is an
   : approach that leads to ABI complexity.  It's part of what makes the
   : Win32 ABI so difficult to work with.  Don't tread that path.  Instead,
   : just change the existing function.

   OK - that is actually what I wanted to do in the first place, but I
   did not know if there were lots of utilities out there that use the
   current disassembler() function, so I did not want to change it.

I doubt anything uses disassembler other than the binutils and gdb.

   : In objdump.c, I think target_data is a poor choice of names.  This is
   : an option to the disassembler.  objdump does many things other than
   : disassemble, so using a word like `target_data' to describe something
   : which is specific to the disassembler is misleading.  Something like
   : `disassembler_options' makes more sense to me.  This should be changed
   : in the suggested documentation as well.

   OK - although I had hoped that this might turn into a more generic
   type of command line switch where the text could be passed to other
   components of objdump, not just its disassembler.

That's an interesting idea.  For most of what objdump does, though,
there is no code which could interpret the option.  It only makes
sense to have a generic option like that in cases where objdump
invokes target specific code.  The only case which comes to mind other
than disassembly is the -p option to print private data.  I could
imagine cases where it would be useful to pass an option to -p, but I
think that generally it would not make sense.  I don't really want to
go down the path of passing a lot of options to -p to control what it
prints.

However, perhaps there is something I am not thinking of.

Ian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
@ 1999-07-01  0:00 Nick Clifton
  1999-07-01  0:00 ` Scott Bambrough
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Clifton @ 1999-07-01  0:00 UTC (permalink / raw)
  To: richard.earnshaw; +Cc: binutils, richard.earnshaw

Hi Richard,

: Personally, I can't stand the APCS register names.  They seem
: illogical to me.

I agree with you 100%.

: Finally, I thought that there was already a command built into gdb
: that could change the register names to the apcs variant if desired;
: this can be put into an init script, so I see little point in the
: need for a command-line option to do this as well.

This option is so that other tools can also change the register name
set being used.  In particualr we have a customer who is insisting on
the APCS name set (fools) for things like objdump and Cygmon.
 
: If we must add this, can't we make it part of the configuration
: options, then a target which uses the current convention doesn't
: change under people's feet.

No - it really needs to be a run time option.  However I am quite
happy to change it so that the standard names are the default and the
APCS names are only selected via a command line option.


Cheers
	Nick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
@ 1999-07-01  0:00 Nick Clifton
  1999-07-01  0:00 ` Ian Lance Taylor
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Clifton @ 1999-07-01  0:00 UTC (permalink / raw)
  To: ian; +Cc: binutils

Hi Ian,

: I do have some comments on the rest of the patch.  I think adding an
: option which can be passed to the disassembler makes sense.  I can
: think of two other cases where that would make sense, both for the
: ix86: getting Intel syntax and disassembling in 16 bit mode.  Both are
: currently done using the -m option to select a machine type, but this
: leads to a meaningless bfd_mach_i386_i386_intel_syntax in
: bfd/cpu-i386.c.
: 
: However, adding a new disassembler2 function does not make sense to
: me.  Adding new functions because you have a new parameter is an
: approach that leads to ABI complexity.  It's part of what makes the
: Win32 ABI so difficult to work with.  Don't tread that path.  Instead,
: just change the existing function.

OK - that is actually what I wanted to do in the first place, but I
did not know if there were lots of utilities out there that use the
current disassembler() function, so I did not want to change it.

: If you must add a new function, a name like disassembler2 is a poor
: choice.  Instead, use a meaningful name, such as, perhaps,
: disassembler_select (that's not very good either, I admit).
: 
: However, in this particular case we should not use a new function at
: all.  Instead, the target specific data should be stored in a field of
: struct disassemble_info.  This easily permits each disassembler to
: examine that field and make its own choices.  After all, the code in
: your disassembler2 function which is ARM specific should not be in
: disassemble.c at all.  It should be in arm-dis.c.

Good point - I will generate a new patch and submit it later on today.

: In objdump.c, I think target_data is a poor choice of names.  This is
: an option to the disassembler.  objdump does many things other than
: disassemble, so using a word like `target_data' to describe something
: which is specific to the disassembler is misleading.  Something like
: `disassembler_options' makes more sense to me.  This should be changed
: in the suggested documentation as well.

OK - although I had hoped that this might turn into a more generic
type of command line switch where the text could be passed to other
components of objdump, not just its disassembler.

Cheers
	Nick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Patch to change ARM register name set
  1999-07-01  0:00 Nick Clifton
  1999-07-01  0:00 ` Richard Earnshaw
@ 1999-07-01  0:00 ` Ian Lance Taylor
  1 sibling, 0 replies; 12+ messages in thread
From: Ian Lance Taylor @ 1999-07-01  0:00 UTC (permalink / raw)
  To: nickc; +Cc: binutils

   Date: Mon, 14 Jun 1999 15:09:19 +0100
   From: Nick Clifton <nickc@cygnus.com>

     Does anyone have any comments on the patch below ?  It changes the
     ARM disassembler so that by default it uses the APCS register naming
     scheme, rather than the ISA register naming scheme.  It also
     introduces a new command line switch to objdump: -M or --target-data,
     which takes an argument which can be processed in a target specific
     manner.  If the target is the ARM then the text of the -M switch is
     checked to see if it is a register name set selector
     ("standard_names" or "apcs_names") and if so it chooses the
     appropriate name set. 

I don't care what ARM register names are used.

I do have some comments on the rest of the patch.  I think adding an
option which can be passed to the disassembler makes sense.  I can
think of two other cases where that would make sense, both for the
ix86: getting Intel syntax and disassembling in 16 bit mode.  Both are
currently done using the -m option to select a machine type, but this
leads to a meaningless bfd_mach_i386_i386_intel_syntax in
bfd/cpu-i386.c.

However, adding a new disassembler2 function does not make sense to
me.  Adding new functions because you have a new parameter is an
approach that leads to ABI complexity.  It's part of what makes the
Win32 ABI so difficult to work with.  Don't tread that path.  Instead,
just change the existing function.

If you must add a new function, a name like disassembler2 is a poor
choice.  Instead, use a meaningful name, such as, perhaps,
disassembler_select (that's not very good either, I admit).

However, in this particular case we should not use a new function at
all.  Instead, the target specific data should be stored in a field of
struct disassemble_info.  This easily permits each disassembler to
examine that field and make its own choices.  After all, the code in
your disassembler2 function which is ARM specific should not be in
disassemble.c at all.  It should be in arm-dis.c.

In objdump.c, I think target_data is a poor choice of names.  This is
an option to the disassembler.  objdump does many things other than
disassemble, so using a word like `target_data' to describe something
which is specific to the disassembler is misleading.  Something like
`disassembler_options' makes more sense to me.  This should be changed
in the suggested documentation as well.

Ian

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~1999-07-01  0:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-07-01  0:00 Patch to change ARM register name set Nick Clifton
1999-07-01  0:00 ` Richard Earnshaw
  -- strict thread matches above, loose matches on Subject: below --
1999-07-01  0:00 Nick Clifton
1999-07-01  0:00 ` Richard Earnshaw
1999-07-01  0:00 ` Ian Lance Taylor
1999-07-01  0:00 Nick Clifton
1999-07-01  0:00 ` Ian Lance Taylor
1999-07-01  0:00 Nick Clifton
1999-07-01  0:00 ` Richard Earnshaw
1999-07-01  0:00 Nick Clifton
1999-07-01  0:00 ` Scott Bambrough
1999-07-01  0:00 Nick Clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).