* [RFA] CFI improvements
@ 2003-05-20 12:21 Michal Ludvig
2003-05-20 12:55 ` Andreas Jaeger
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Michal Ludvig @ 2003-05-20 12:21 UTC (permalink / raw)
To: binutils; +Cc: Alan Modra
[-- Attachment #1: Type: text/plain, Size: 739 bytes --]
Hi,
the attached patch adds some new features to the CFI engine:
- register names can start with '%' as requested by GCC guys.
- CIEs are not duplicated anymore but instead reused if possible.
- tc_cfi_init() is moved from main() to dot_cfi_startproc() - otherwise
it didn't set correct values on amd64 because the output format wasn't
yet known in main().
- .cfi_startproc takes optional parameter 'simple' that disables
emitting of architecture-dependent initial instructions.
- testsuite is modified to pass with CIE optimalization.
I hope I haven't introduce any new C89 incompatibility (sorry for that one).
Comments? Approvals?
Michal Ludvig
--
* SuSE CR, s.r.o * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz
[-- Attachment #2: gas-cfi-cvs-1.diff --]
[-- Type: text/plain, Size: 15822 bytes --]
2003-05-20 Michal Ludvig <mludvig@suse.cz>
* as.c (main): Remove tc_cfi_init().
* dw2gencfi.c (cfi_parse_arg): Allow regnames beginning
with '%'.
(cfi_make_insn): Handle CFA_register.
(cfi_output_insn): Ditto.
(dot_cfi): Ditto.
(cfi_get_label): Add 'simple' modifier to .cfi_startproc.
(dot_cfi_endproc): Reuse already emitted CIEs.
* testsuite/gas/cfi/cfi-i386.d: New pattern.
* testsuite/gas/cfi/cfi-x86-64.d: Ditto.
Index: as.c
===================================================================
RCS file: /cvs/src/src/gas/as.c,v
retrieving revision 1.43
diff -u -p -r1.43 as.c
--- as.c 20 May 2003 07:58:06 -0000 1.43
+++ as.c 20 May 2003 12:04:30 -0000
@@ -837,10 +837,6 @@ main (argc, argv)
bfd_set_error_program_name (myname);
#endif
-#ifdef TARGET_USE_CFIPOP
- tc_cfi_init ();
-#endif
-
#ifdef USE_EMULATIONS
select_emulation_mode (argc, argv);
#endif
Index: dw2gencfi.c
===================================================================
RCS file: /cvs/src/src/gas/dw2gencfi.c,v
retrieving revision 1.2
diff -u -p -r1.2 dw2gencfi.c
--- dw2gencfi.c 20 May 2003 11:35:45 -0000 1.2
+++ dw2gencfi.c 20 May 2003 12:04:30 -0000
@@ -23,24 +23,55 @@
#include "as.h"
#include "dw2gencfi.h"
+struct cie_entry
+{
+ unsigned long offset;
+ size_t size;
+ void *data;
+ struct cie_entry *next;
+};
+
+struct cfi_data
+{
+ enum cfi_insn insn;
+ long param[2];
+ struct cfi_data *next;
+};
+
+struct cfi_info
+{
+ addressT start_address;
+ addressT end_address;
+ addressT last_address;
+ const char *labelname;
+ struct cfi_data *data;
+ struct cfi_info *next;
+};
+
+/* Current open CFI entry. */
+static struct cfi_info *cfi_info;
+
+/* List of CIEs so that they could be reused. */
+static struct cie_entry *cie_root;
+
/* Current target config. */
static struct cfi_config current_config;
/* This is the main entry point to the CFI machinery. */
static void dot_cfi (int arg);
-const pseudo_typeS cfi_pseudo_table[] =
- {
- { "cfi_verbose", dot_cfi, CFI_verbose },
- { "cfi_startproc", dot_cfi, CFI_startproc },
- { "cfi_endproc", dot_cfi, CFI_endproc },
- { "cfi_def_cfa", dot_cfi, CFA_def_cfa },
- { "cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register },
- { "cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset },
- { "cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset },
- { "cfi_offset", dot_cfi, CFA_offset },
- { NULL, NULL, 0 }
- };
+const pseudo_typeS cfi_pseudo_table[] = {
+ {"cfi_verbose", dot_cfi, CFI_verbose},
+ {"cfi_startproc", dot_cfi, CFI_startproc},
+ {"cfi_endproc", dot_cfi, CFI_endproc},
+ {"cfi_def_cfa", dot_cfi, CFA_def_cfa},
+ {"cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register},
+ {"cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset},
+ {"cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset},
+ {"cfi_offset", dot_cfi, CFA_offset},
+ {"cfi_register", dot_cfi, CFA_register},
+ {NULL, NULL, 0}
+};
static const char *
cfi_insn_str (enum cfi_insn insn)
@@ -90,25 +121,6 @@ cfi_insn_str (enum cfi_insn insn)
return "CFA_unknown";
}
-struct cfi_data
-{
- enum cfi_insn insn;
- long param[2];
- struct cfi_data *next;
-};
-
-struct cfi_info
-{
- addressT start_address;
- addressT end_address;
- addressT last_address;
- const char *labelname;
- struct cfi_data *data;
- struct cfi_info *next;
-};
-
-static struct cfi_info *cfi_info;
-
static struct cfi_data *
alloc_cfi_data (void)
{
@@ -138,7 +150,9 @@ cfi_parse_arg (long *param, int resolver
retval = 1;
}
#ifdef tc_regname_to_dw2regnum
- else if (resolvereg && (is_name_beginner (*input_line_pointer)))
+ else if (resolvereg && ((is_name_beginner (*input_line_pointer)) ||
+ (*input_line_pointer == '%' &&
+ is_name_beginner (*(++input_line_pointer)))))
{
char *name, c, *p;
@@ -271,6 +285,21 @@ cfi_make_insn (int arg)
}
break;
+ case CFA_register:
+ if (cfi_parse_reg (¶m[0]) < 0)
+ {
+ as_bad (_("first argument to %s is not a register"),
+ cfi_insn_str (arg));
+ return;
+ }
+ if (cfi_parse_reg (¶m[1]) < 0)
+ {
+ as_bad (_("second argument to %s is not a register"),
+ cfi_insn_str (arg));
+ return;
+ }
+ break;
+
/* Instructions that take one register argument. */
case CFA_def_cfa_register:
if (cfi_parse_reg (¶m[0]) < 0)
@@ -336,20 +365,33 @@ cfi_get_label (void)
static void
dot_cfi_startproc (void)
{
+ char *simple = "simple";
+
if (cfi_info)
{
as_bad (_("previous CFI entry not closed (missing .cfi_endproc)"));
return;
}
+#if defined(TARGET_USE_CFIPOP)
+ /* Because this file is linked even for architectures that
+ don't use CFI, we must wrap this call. */
+ if (current_config.addr_length == 0)
+ tc_cfi_init ();
+#endif
+
cfi_info = alloc_cfi_info ();
cfi_info->start_address = frag_now_fix ();
cfi_info->last_address = cfi_info->start_address;
cfi_info->labelname = S_GET_NAME (cfi_get_label ());
+ SKIP_WHITESPACE ();
#ifdef tc_cfi_frame_initial_instructions
- tc_cfi_frame_initial_instructions ();
+ if (strncmp (simple, input_line_pointer, strlen (simple)) != 0)
+ tc_cfi_frame_initial_instructions ();
+ else
+ input_line_pointer += strlen (simple);
#endif
}
@@ -357,6 +399,8 @@ dot_cfi_startproc (void)
((insn >= CFA_set_loc && insn <= CFA_advance_loc4) \
|| insn == CFA_advance_loc)
+/* Output CFI instructions to the file. */
+
enum data_types
{
t_ascii = 0,
@@ -368,8 +412,6 @@ enum data_types
t_sleb128 = 0x11
};
-/* Output CFI instructions to the file. */
-
static int
output_data (char **p, unsigned long *size, enum data_types type, long value)
{
@@ -393,7 +435,8 @@ output_data (char **p, unsigned long *si
ret_size = 8;
break;
default:
- as_warn (_("unknown type %d"), type);
+ /* This should never happen - throw an internal error. */
+ as_fatal (_("unknown type %d"), type);
return 0;
}
@@ -434,7 +477,7 @@ output_data (char **p, unsigned long *si
value);
break;
default:
- as_warn ("unknown type %d", type);
+ as_fatal (_("unknown type %d"), type);
return 0;
}
@@ -486,7 +529,8 @@ cfi_output_insn (struct cfi_data *data,
case CFA_def_cfa:
if (verbose)
- printf ("\t# CFA_def_cfa(%ld,%ld)\n", data->param[0], data->param[1]);
+ printf ("\t# CFA_def_cfa(%ld,%ld)\n",
+ data->param[0], data->param[1]);
output_data (pbuf, buf_size, t_byte, CFA_def_cfa);
output_data (pbuf, buf_size, t_uleb128, data->param[0]);
output_data (pbuf, buf_size, t_uleb128, data->param[1]);
@@ -518,6 +562,15 @@ cfi_output_insn (struct cfi_data *data,
data->param[1] / current_config.data_align);
break;
+ case CFA_register:
+ if (verbose)
+ printf ("\t# %s(%ld,%ld)\n", cfi_insn_str (data->insn),
+ data->param[0], data->param[1]);
+ output_data (pbuf, buf_size, t_byte, CFA_register);
+ output_data (pbuf, buf_size, t_uleb128, data->param[0]);
+ output_data (pbuf, buf_size, t_uleb128, data->param[1]);
+ break;
+
case CFA_nop:
if (verbose)
printf ("\t# CFA_nop\n");
@@ -538,9 +591,10 @@ static void
dot_cfi_endproc (void)
{
struct cfi_data *data_ptr;
+ struct cie_entry *cie_ptr;
char *cie_buf, *fde_buf, *pbuf, *where;
- unsigned long buf_size, cie_size, fde_size, last_cie_offset;
- unsigned long fde_initloc_offset, fde_len_offset;
+ unsigned long buf_size, cie_size, fde_size, last_cie_offset;
+ unsigned long fde_initloc_offset, fde_len_offset, fde_offset;
void *saved_seg, *cfi_seg;
expressionS exp;
@@ -598,8 +653,51 @@ dot_cfi_endproc (void)
/* OK, we built the CIE. Let's write it to the file... */
last_cie_offset = frag_now_fix ();
- where = (unsigned char *) frag_more (cie_size);
- memcpy (where, cie_buf, cie_size);
+
+ /* Check if we have already emitted the exactly same CIE.
+ If yes then use its offset instead and don't put out
+ the new one. */
+ cie_ptr = cie_root;
+ while (cie_ptr)
+ {
+ if (cie_ptr->size == cie_size - 4 &&
+ memcmp (cie_ptr->data, cie_buf + 4, cie_ptr->size) == 0)
+ break;
+ cie_ptr = cie_ptr->next;
+ }
+
+ /* If we have found the same CIE, use it... */
+ if (cie_ptr)
+ {
+ if (verbose)
+ printf ("# Duplicate CIE found. Previous is at offset %lu\n",
+ cie_ptr->offset);
+ last_cie_offset = cie_ptr->offset;
+ }
+ else
+ {
+ /* Otherwise join this CIE to the list. */
+ where = (unsigned char *) frag_more (cie_size);
+ memcpy (where, cie_buf, cie_size);
+ if (cie_root)
+ {
+ cie_ptr = cie_root;
+ while (cie_ptr->next)
+ cie_ptr = cie_ptr->next;
+ cie_ptr->next = calloc (sizeof (struct cie_entry), 1);
+ cie_ptr = cie_ptr->next;
+ }
+ else
+ {
+ cie_root = calloc (sizeof (struct cie_entry), 1);
+ cie_ptr = cie_root;
+ }
+
+ cie_ptr->size = cie_size - 4;
+ cie_ptr->data = calloc (cie_ptr->size, 1);
+ cie_ptr->offset = last_cie_offset;
+ memcpy (cie_ptr->data, cie_buf + 4, cie_ptr->size);
+ }
/* Clean up. */
free (cie_buf);
@@ -609,6 +707,9 @@ dot_cfi_endproc (void)
pbuf = fde_buf;
buf_size = 1024;
+ /* Offset of this FDE in current fragment. */
+ fde_offset = frag_now_fix ();
+
if (verbose)
{
printf ("# FDE: start=0x%lx, end=0x%lx, delta=%d\n",
@@ -623,10 +724,10 @@ dot_cfi_endproc (void)
buf_size -= 4;
/* CIE pointer - offset from here. */
- output_data (&pbuf, &buf_size, t_long, cie_size + 4);
+ output_data (&pbuf, &buf_size, t_long, fde_offset - last_cie_offset + 4);
/* FDE initial location - this must be set relocatable! */
- fde_initloc_offset = pbuf - fde_buf;
+ fde_initloc_offset = pbuf - fde_buf + fde_offset;
output_data (&pbuf, &buf_size, current_config.addr_length,
cfi_info->start_address);
@@ -648,9 +749,6 @@ dot_cfi_endproc (void)
buf_size = 4;
output_data (&pbuf, &buf_size, t_long, fde_size - 4);
- /* Adjust initloc offset. */
- fde_initloc_offset += frag_now_fix ();
-
/* Copy FDE to objfile. */
where = (unsigned char *) frag_more (fde_size);
memcpy (where, fde_buf, fde_size);
@@ -691,6 +789,7 @@ dot_cfi (int arg)
case CFA_def_cfa_register:
case CFA_def_cfa_offset:
case CFA_offset:
+ case CFA_register:
case CFI_adjust_cfa_offset:
cfi_make_insn (arg);
break;
Index: testsuite/gas/cfi/cfi-i386.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/cfi/cfi-i386.d,v
retrieving revision 1.1
diff -u -p -r1.1 cfi-i386.d
--- testsuite/gas/cfi/cfi-i386.d 20 May 2003 08:01:19 -0000 1.1
+++ testsuite/gas/cfi/cfi-i386.d 20 May 2003 12:04:30 -0000
@@ -1,6 +1,5 @@
#readelf: -wf
#name: CFI on i386
-
The section .eh_frame contains:
00000000 00000010 00000000 CIE
@@ -22,19 +21,7 @@ The section .eh_frame contains:
DW_CFA_def_cfa_offset: 4
DW_CFA_nop
-0000002c 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-00000040 00000018 00000018 FDE cie=0000002c pc=00000012..0000001f
+0000002c 00000018 00000030 FDE cie=00000000 pc=00000012..0000001f
DW_CFA_advance_loc: 1 to 00000013
DW_CFA_def_cfa_offset: 8
DW_CFA_offset: r6 at cfa-8
@@ -44,56 +31,20 @@ The section .eh_frame contains:
DW_CFA_def_cfa_reg: r7
DW_CFA_nop
-0000005c 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-00000070 00000014 00000018 FDE cie=0000005c pc=0000001f..0000002f
+00000048 00000014 0000004c FDE cie=00000000 pc=0000001f..0000002f
DW_CFA_advance_loc: 2 to 00000021
DW_CFA_def_cfa_reg: r1
DW_CFA_advance_loc: 13 to 0000002e
DW_CFA_def_cfa: r7 ofs 4
DW_CFA_nop
-00000088 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-0000009c 00000010 00000018 FDE cie=00000088 pc=0000002f..00000035
+00000060 00000010 00000064 FDE cie=00000000 pc=0000002f..00000035
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
-000000b0 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-000000c4 00000010 00000018 FDE cie=000000b0 pc=00000035..00000044
+00000074 00000010 00000078 FDE cie=00000000 pc=00000035..00000044
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
Index: testsuite/gas/cfi/cfi-x86_64.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/cfi/cfi-x86_64.d,v
retrieving revision 1.1
diff -u -p -r1.1 cfi-x86_64.d
--- testsuite/gas/cfi/cfi-x86_64.d 20 May 2003 08:01:19 -0000 1.1
+++ testsuite/gas/cfi/cfi-x86_64.d 20 May 2003 12:04:30 -0000
@@ -26,23 +26,7 @@ The section .eh_frame contains:
DW_CFA_def_cfa_offset: 8
DW_CFA_nop
-00000038 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-00000050 00000024 0000001c FDE cie=00000038 pc=00000000..0000000f
+00000038 00000024 0000003c FDE cie=00000000 pc=00000000..0000000f
DW_CFA_advance_loc: 1 to 00000001
DW_CFA_def_cfa_offset: 16
DW_CFA_offset: r6 at cfa-16
@@ -55,23 +39,7 @@ The section .eh_frame contains:
DW_CFA_nop
DW_CFA_nop
-00000078 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-00000090 0000001c 0000001c FDE cie=00000078 pc=00000000..00000013
+00000060 0000001c 00000064 FDE cie=00000000 pc=00000000..00000013
DW_CFA_advance_loc: 3 to 00000003
DW_CFA_def_cfa_reg: r12
DW_CFA_advance_loc: 15 to 00000012
@@ -79,23 +47,7 @@ The section .eh_frame contains:
DW_CFA_nop
DW_CFA_nop
-000000b0 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-000000c8 0000001c 0000001c FDE cie=000000b0 pc=00000000..00000006
+00000080 0000001c 00000084 FDE cie=00000000 pc=00000000..00000006
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
@@ -105,23 +57,7 @@ The section .eh_frame contains:
DW_CFA_nop
DW_CFA_nop
-000000e8 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-00000100 0000001c 0000001c FDE cie=000000e8 pc=00000000..00000012
+000000a0 0000001c 000000a4 FDE cie=00000000 pc=00000000..00000012
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 12:21 [RFA] CFI improvements Michal Ludvig
@ 2003-05-20 12:55 ` Andreas Jaeger
2003-05-20 12:57 ` Alan Modra
2003-05-20 23:54 ` Hans-Peter Nilsson
2 siblings, 0 replies; 15+ messages in thread
From: Andreas Jaeger @ 2003-05-20 12:55 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils, Alan Modra
Michal Ludvig <mludvig@suse.cz> writes:
> Hi,
> the attached patch adds some new features to the CFI engine:
> - register names can start with '%' as requested by GCC guys.
> - CIEs are not duplicated anymore but instead reused if possible.
> - tc_cfi_init() is moved from main() to dot_cfi_startproc() -
> otherwise it didn't set correct values on amd64 because the output
> format wasn't yet known in main().
> - .cfi_startproc takes optional parameter 'simple' that disables
> emitting of architecture-dependent initial instructions.
> - testsuite is modified to pass with CIE optimalization.
>
> I hope I haven't introduce any new C89 incompatibility (sorry for that one).
>
> Comments? Approvals?
>
> Michal Ludvig
> Index: dw2gencfi.c
> ===================================================================
> RCS file: /cvs/src/src/gas/dw2gencfi.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 dw2gencfi.c
> --- dw2gencfi.c 20 May 2003 11:35:45 -0000 1.2
> +++ dw2gencfi.c 20 May 2003 12:04:30 -0000
> @@ -23,24 +23,55 @@
> #include "as.h"
> #include "dw2gencfi.h"
>
> +struct cie_entry
> +{
> + unsigned long offset;
> + size_t size;
> + void *data;
> + struct cie_entry *next;
> +};
> +
> +struct cfi_data
> +{
> + enum cfi_insn insn;
> + long param[2];
> + struct cfi_data *next;
> +};
> +
> +struct cfi_info
> +{
> + addressT start_address;
> + addressT end_address;
> + addressT last_address;
> + const char *labelname;
> + struct cfi_data *data;
> + struct cfi_info *next;
> +};
> +
> +/* Current open CFI entry. */
> +static struct cfi_info *cfi_info;
> +
> +/* List of CIEs so that they could be reused. */
> +static struct cie_entry *cie_root;
> +
> /* Current target config. */
> static struct cfi_config current_config;
>
> /* This is the main entry point to the CFI machinery. */
> static void dot_cfi (int arg);
>
> -const pseudo_typeS cfi_pseudo_table[] =
> - {
> - { "cfi_verbose", dot_cfi, CFI_verbose },
> - { "cfi_startproc", dot_cfi, CFI_startproc },
> - { "cfi_endproc", dot_cfi, CFI_endproc },
> - { "cfi_def_cfa", dot_cfi, CFA_def_cfa },
> - { "cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register },
> - { "cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset },
> - { "cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset },
> - { "cfi_offset", dot_cfi, CFA_offset },
> - { NULL, NULL, 0 }
> - };
> +const pseudo_typeS cfi_pseudo_table[] = {
> + {"cfi_verbose", dot_cfi, CFI_verbose},
> + {"cfi_startproc", dot_cfi, CFI_startproc},
> + {"cfi_endproc", dot_cfi, CFI_endproc},
> + {"cfi_def_cfa", dot_cfi, CFA_def_cfa},
> + {"cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register},
> + {"cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset},
> + {"cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset},
> + {"cfi_offset", dot_cfi, CFA_offset},
> + {"cfi_register", dot_cfi, CFA_register},
> + {NULL, NULL, 0}
> +};
These look like whitespace changes. Alan has done a number of
formatting changes, can you incorporate them cleanly into your patch
and then resend the patch? Reverting Alan's formatting changes is not
the right way IMO.
> [...]
> + char *simple = "simple";
const char?
Andreas
--
Andreas Jaeger
SuSE Labs aj@suse.de
private aj@arthur.inka.de
http://www.suse.de/~aj
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 12:21 [RFA] CFI improvements Michal Ludvig
2003-05-20 12:55 ` Andreas Jaeger
@ 2003-05-20 12:57 ` Alan Modra
2003-05-20 13:10 ` Michal Ludvig
2003-05-20 13:55 ` Michal Ludvig
2003-05-20 23:54 ` Hans-Peter Nilsson
2 siblings, 2 replies; 15+ messages in thread
From: Alan Modra @ 2003-05-20 12:57 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils
On Tue, May 20, 2003 at 02:21:18PM +0200, Michal Ludvig wrote:
> -const pseudo_typeS cfi_pseudo_table[] =
> - {
> - { "cfi_verbose", dot_cfi, CFI_verbose },
> - { "cfi_startproc", dot_cfi, CFI_startproc },
> - { "cfi_endproc", dot_cfi, CFI_endproc },
> - { "cfi_def_cfa", dot_cfi, CFA_def_cfa },
> - { "cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register },
> - { "cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset },
> - { "cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset },
> - { "cfi_offset", dot_cfi, CFA_offset },
> - { NULL, NULL, 0 }
> - };
> +const pseudo_typeS cfi_pseudo_table[] = {
> + {"cfi_verbose", dot_cfi, CFI_verbose},
> + {"cfi_startproc", dot_cfi, CFI_startproc},
> + {"cfi_endproc", dot_cfi, CFI_endproc},
> + {"cfi_def_cfa", dot_cfi, CFA_def_cfa},
> + {"cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register},
> + {"cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset},
> + {"cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset},
> + {"cfi_offset", dot_cfi, CFA_offset},
> + {"cfi_register", dot_cfi, CFA_register},
> + {NULL, NULL, 0}
> +};
Did you mean to undo my formatting fixes?
> - else if (resolvereg && (is_name_beginner (*input_line_pointer)))
> + else if (resolvereg && ((is_name_beginner (*input_line_pointer)) ||
> + (*input_line_pointer == '%' &&
> + is_name_beginner (*(++input_line_pointer)))))
Please fix the formatting errors here.
> + if (cie_ptr->size == cie_size - 4 &&
> + memcmp (cie_ptr->data, cie_buf + 4, cie_ptr->size) == 0)
Again.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 12:57 ` Alan Modra
@ 2003-05-20 13:10 ` Michal Ludvig
2003-05-20 13:55 ` Michal Ludvig
1 sibling, 0 replies; 15+ messages in thread
From: Michal Ludvig @ 2003-05-20 13:10 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
Alan Modra told me that:
> On Tue, May 20, 2003 at 02:21:18PM +0200, Michal Ludvig wrote:
>
>>-const pseudo_typeS cfi_pseudo_table[] =
>>- {
>>- { "cfi_verbose", dot_cfi, CFI_verbose },
>>- { "cfi_startproc", dot_cfi, CFI_startproc },
>>- { "cfi_endproc", dot_cfi, CFI_endproc },
>>- { "cfi_def_cfa", dot_cfi, CFA_def_cfa },
>>- { "cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register },
>>- { "cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset },
>>- { "cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset },
>>- { "cfi_offset", dot_cfi, CFA_offset },
>>- { NULL, NULL, 0 }
>>- };
>>+const pseudo_typeS cfi_pseudo_table[] = {
>>+ {"cfi_verbose", dot_cfi, CFI_verbose},
>>+ {"cfi_startproc", dot_cfi, CFI_startproc},
>>+ {"cfi_endproc", dot_cfi, CFI_endproc},
>>+ {"cfi_def_cfa", dot_cfi, CFA_def_cfa},
>>+ {"cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register},
>>+ {"cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset},
>>+ {"cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset},
>>+ {"cfi_offset", dot_cfi, CFA_offset},
>>+ {"cfi_register", dot_cfi, CFA_register},
>>+ {NULL, NULL, 0}
>>+};
>
> Did you mean to undo my formatting fixes?
Oops sorry - I was saying me don't forget to change it before submitting
and ... finally I forgot, of course. I didn't mean to revert your
formatting ;-)
BTW how did you format it? By hand? I run indent without parameters and
got what I submitted...
I'll resend the reindented patch. Or are there some other things to fix
except the formating?
Michal Ludvig
--
* SuSE CR, s.r.o * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 12:57 ` Alan Modra
2003-05-20 13:10 ` Michal Ludvig
@ 2003-05-20 13:55 ` Michal Ludvig
2003-05-20 14:18 ` Alan Modra
1 sibling, 1 reply; 15+ messages in thread
From: Michal Ludvig @ 2003-05-20 13:55 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]
Alan Modra told me that:
> On Tue, May 20, 2003 at 02:21:18PM +0200, Michal Ludvig wrote:
>
>>-const pseudo_typeS cfi_pseudo_table[] =
>>- {
>>- { "cfi_verbose", dot_cfi, CFI_verbose },
>>- { "cfi_startproc", dot_cfi, CFI_startproc },
>>- { "cfi_endproc", dot_cfi, CFI_endproc },
>>- { "cfi_def_cfa", dot_cfi, CFA_def_cfa },
>>- { "cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register },
>>- { "cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset },
>>- { "cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset },
>>- { "cfi_offset", dot_cfi, CFA_offset },
>>- { NULL, NULL, 0 }
>>- };
>>+const pseudo_typeS cfi_pseudo_table[] = {
>>+ {"cfi_verbose", dot_cfi, CFI_verbose},
>>+ {"cfi_startproc", dot_cfi, CFI_startproc},
>>+ {"cfi_endproc", dot_cfi, CFI_endproc},
>>+ {"cfi_def_cfa", dot_cfi, CFA_def_cfa},
>>+ {"cfi_def_cfa_register", dot_cfi, CFA_def_cfa_register},
>>+ {"cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset},
>>+ {"cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset},
>>+ {"cfi_offset", dot_cfi, CFA_offset},
>>+ {"cfi_register", dot_cfi, CFA_register},
>>+ {NULL, NULL, 0}
>>+};
'indent' is still reverting this... Anyway I manually kept it your way
in the diff.
>>- else if (resolvereg && (is_name_beginner (*input_line_pointer)))
>>+ else if (resolvereg && ((is_name_beginner (*input_line_pointer)) ||
>>+ (*input_line_pointer == '%' &&
>>+ is_name_beginner (*(++input_line_pointer)))))
>
> Please fix the formatting errors here.
Fixed.
>>+ if (cie_ptr->size == cie_size - 4 &&
>>+ memcmp (cie_ptr->data, cie_buf + 4, cie_ptr->size) == 0)
>
> Again.
Fixed.
OK to commit?
Michal Ludvig
--
* SuSE CR, s.r.o * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz
[-- Attachment #2: gas-cfi-cvs-2.diff --]
[-- Type: text/plain, Size: 15039 bytes --]
2003-05-20 Michal Ludvig <mludvig@suse.cz>
* as.c (main): Remove tc_cfi_init().
* dw2gencfi.c (cfi_parse_arg): Allow regnames beginning
with '%'.
(cfi_pseudo_table): Add "cfi_register" entry.
(cfi_make_insn): Handle CFA_register.
(cfi_output_insn): Ditto.
(dot_cfi): Ditto.
(cfi_get_label): Add 'simple' modifier to .cfi_startproc.
(dot_cfi_endproc): Reuse already emitted CIEs.
* testsuite/gas/cfi/cfi-i386.d: New pattern.
* testsuite/gas/cfi/cfi-x86-64.d: Ditto.
Index: as.c
===================================================================
RCS file: /cvs/src/src/gas/as.c,v
retrieving revision 1.43
diff -u -p -r1.43 as.c
--- as.c 20 May 2003 07:58:06 -0000 1.43
+++ as.c 20 May 2003 13:44:34 -0000
@@ -837,10 +837,6 @@ main (argc, argv)
bfd_set_error_program_name (myname);
#endif
-#ifdef TARGET_USE_CFIPOP
- tc_cfi_init ();
-#endif
-
#ifdef USE_EMULATIONS
select_emulation_mode (argc, argv);
#endif
Index: dw2gencfi.c
===================================================================
RCS file: /cvs/src/src/gas/dw2gencfi.c,v
retrieving revision 1.2
diff -u -p -r1.2 dw2gencfi.c
--- dw2gencfi.c 20 May 2003 11:35:45 -0000 1.2
+++ dw2gencfi.c 20 May 2003 13:44:34 -0000
@@ -23,6 +23,37 @@
#include "as.h"
#include "dw2gencfi.h"
+struct cie_entry
+{
+ unsigned long offset;
+ size_t size;
+ void *data;
+ struct cie_entry *next;
+};
+
+struct cfi_data
+{
+ enum cfi_insn insn;
+ long param[2];
+ struct cfi_data *next;
+};
+
+struct cfi_info
+{
+ addressT start_address;
+ addressT end_address;
+ addressT last_address;
+ const char *labelname;
+ struct cfi_data *data;
+ struct cfi_info *next;
+};
+
+/* Current open CFI entry. */
+static struct cfi_info *cfi_info;
+
+/* List of CIEs so that they could be reused. */
+static struct cie_entry *cie_root;
+
/* Current target config. */
static struct cfi_config current_config;
@@ -39,6 +70,7 @@ const pseudo_typeS cfi_pseudo_table[] =
{ "cfi_def_cfa_offset", dot_cfi, CFA_def_cfa_offset },
{ "cfi_adjust_cfa_offset", dot_cfi, CFI_adjust_cfa_offset },
{ "cfi_offset", dot_cfi, CFA_offset },
+ { "cfi_register", dot_cfi, CFA_register },
{ NULL, NULL, 0 }
};
@@ -90,25 +122,6 @@ cfi_insn_str (enum cfi_insn insn)
return "CFA_unknown";
}
-struct cfi_data
-{
- enum cfi_insn insn;
- long param[2];
- struct cfi_data *next;
-};
-
-struct cfi_info
-{
- addressT start_address;
- addressT end_address;
- addressT last_address;
- const char *labelname;
- struct cfi_data *data;
- struct cfi_info *next;
-};
-
-static struct cfi_info *cfi_info;
-
static struct cfi_data *
alloc_cfi_data (void)
{
@@ -138,7 +151,9 @@ cfi_parse_arg (long *param, int resolver
retval = 1;
}
#ifdef tc_regname_to_dw2regnum
- else if (resolvereg && (is_name_beginner (*input_line_pointer)))
+ else if (resolvereg && ((is_name_beginner (*input_line_pointer))
+ || (*input_line_pointer == '%'
+ && is_name_beginner (*(++input_line_pointer)))))
{
char *name, c, *p;
@@ -271,6 +286,21 @@ cfi_make_insn (int arg)
}
break;
+ case CFA_register:
+ if (cfi_parse_reg (¶m[0]) < 0)
+ {
+ as_bad (_("first argument to %s is not a register"),
+ cfi_insn_str (arg));
+ return;
+ }
+ if (cfi_parse_reg (¶m[1]) < 0)
+ {
+ as_bad (_("second argument to %s is not a register"),
+ cfi_insn_str (arg));
+ return;
+ }
+ break;
+
/* Instructions that take one register argument. */
case CFA_def_cfa_register:
if (cfi_parse_reg (¶m[0]) < 0)
@@ -336,20 +366,33 @@ cfi_get_label (void)
static void
dot_cfi_startproc (void)
{
+ const char *simple = "simple";
+
if (cfi_info)
{
as_bad (_("previous CFI entry not closed (missing .cfi_endproc)"));
return;
}
+#if defined(TARGET_USE_CFIPOP)
+ /* Because this file is linked even for architectures that
+ don't use CFI, we must wrap this call. */
+ if (current_config.addr_length == 0)
+ tc_cfi_init ();
+#endif
+
cfi_info = alloc_cfi_info ();
cfi_info->start_address = frag_now_fix ();
cfi_info->last_address = cfi_info->start_address;
cfi_info->labelname = S_GET_NAME (cfi_get_label ());
+ SKIP_WHITESPACE ();
#ifdef tc_cfi_frame_initial_instructions
- tc_cfi_frame_initial_instructions ();
+ if (strncmp (simple, input_line_pointer, strlen (simple)) != 0)
+ tc_cfi_frame_initial_instructions ();
+ else
+ input_line_pointer += strlen (simple);
#endif
}
@@ -357,6 +400,8 @@ dot_cfi_startproc (void)
((insn >= CFA_set_loc && insn <= CFA_advance_loc4) \
|| insn == CFA_advance_loc)
+/* Output CFI instructions to the file. */
+
enum data_types
{
t_ascii = 0,
@@ -368,8 +413,6 @@ enum data_types
t_sleb128 = 0x11
};
-/* Output CFI instructions to the file. */
-
static int
output_data (char **p, unsigned long *size, enum data_types type, long value)
{
@@ -393,7 +436,8 @@ output_data (char **p, unsigned long *si
ret_size = 8;
break;
default:
- as_warn (_("unknown type %d"), type);
+ /* This should never happen - throw an internal error. */
+ as_fatal (_("unknown type %d"), type);
return 0;
}
@@ -434,7 +478,7 @@ output_data (char **p, unsigned long *si
value);
break;
default:
- as_warn ("unknown type %d", type);
+ as_fatal (_("unknown type %d"), type);
return 0;
}
@@ -486,7 +530,8 @@ cfi_output_insn (struct cfi_data *data,
case CFA_def_cfa:
if (verbose)
- printf ("\t# CFA_def_cfa(%ld,%ld)\n", data->param[0], data->param[1]);
+ printf ("\t# CFA_def_cfa(%ld,%ld)\n",
+ data->param[0], data->param[1]);
output_data (pbuf, buf_size, t_byte, CFA_def_cfa);
output_data (pbuf, buf_size, t_uleb128, data->param[0]);
output_data (pbuf, buf_size, t_uleb128, data->param[1]);
@@ -518,6 +563,15 @@ cfi_output_insn (struct cfi_data *data,
data->param[1] / current_config.data_align);
break;
+ case CFA_register:
+ if (verbose)
+ printf ("\t# %s(%ld,%ld)\n", cfi_insn_str (data->insn),
+ data->param[0], data->param[1]);
+ output_data (pbuf, buf_size, t_byte, CFA_register);
+ output_data (pbuf, buf_size, t_uleb128, data->param[0]);
+ output_data (pbuf, buf_size, t_uleb128, data->param[1]);
+ break;
+
case CFA_nop:
if (verbose)
printf ("\t# CFA_nop\n");
@@ -538,9 +592,10 @@ static void
dot_cfi_endproc (void)
{
struct cfi_data *data_ptr;
+ struct cie_entry *cie_ptr;
char *cie_buf, *fde_buf, *pbuf, *where;
- unsigned long buf_size, cie_size, fde_size, last_cie_offset;
- unsigned long fde_initloc_offset, fde_len_offset;
+ unsigned long buf_size, cie_size, fde_size, last_cie_offset;
+ unsigned long fde_initloc_offset, fde_len_offset, fde_offset;
void *saved_seg, *cfi_seg;
expressionS exp;
@@ -598,8 +653,51 @@ dot_cfi_endproc (void)
/* OK, we built the CIE. Let's write it to the file... */
last_cie_offset = frag_now_fix ();
- where = (unsigned char *) frag_more (cie_size);
- memcpy (where, cie_buf, cie_size);
+
+ /* Check if we have already emitted the exactly same CIE.
+ If yes then use its offset instead and don't put out
+ the new one. */
+ cie_ptr = cie_root;
+ while (cie_ptr)
+ {
+ if (cie_ptr->size == cie_size - 4 &&
+ memcmp (cie_ptr->data, cie_buf + 4, cie_ptr->size) == 0)
+ break;
+ cie_ptr = cie_ptr->next;
+ }
+
+ /* If we have found the same CIE, use it... */
+ if (cie_ptr)
+ {
+ if (verbose)
+ printf ("# Duplicate CIE found. Previous is at offset %lu\n",
+ cie_ptr->offset);
+ last_cie_offset = cie_ptr->offset;
+ }
+ else
+ {
+ /* Otherwise join this CIE to the list. */
+ where = (unsigned char *) frag_more (cie_size);
+ memcpy (where, cie_buf, cie_size);
+ if (cie_root)
+ {
+ cie_ptr = cie_root;
+ while (cie_ptr->next)
+ cie_ptr = cie_ptr->next;
+ cie_ptr->next = calloc (sizeof (struct cie_entry), 1);
+ cie_ptr = cie_ptr->next;
+ }
+ else
+ {
+ cie_root = calloc (sizeof (struct cie_entry), 1);
+ cie_ptr = cie_root;
+ }
+
+ cie_ptr->size = cie_size - 4;
+ cie_ptr->data = calloc (cie_ptr->size, 1);
+ cie_ptr->offset = last_cie_offset;
+ memcpy (cie_ptr->data, cie_buf + 4, cie_ptr->size);
+ }
/* Clean up. */
free (cie_buf);
@@ -609,6 +707,9 @@ dot_cfi_endproc (void)
pbuf = fde_buf;
buf_size = 1024;
+ /* Offset of this FDE in current fragment. */
+ fde_offset = frag_now_fix ();
+
if (verbose)
{
printf ("# FDE: start=0x%lx, end=0x%lx, delta=%d\n",
@@ -623,10 +724,10 @@ dot_cfi_endproc (void)
buf_size -= 4;
/* CIE pointer - offset from here. */
- output_data (&pbuf, &buf_size, t_long, cie_size + 4);
+ output_data (&pbuf, &buf_size, t_long, fde_offset - last_cie_offset + 4);
/* FDE initial location - this must be set relocatable! */
- fde_initloc_offset = pbuf - fde_buf;
+ fde_initloc_offset = pbuf - fde_buf + fde_offset;
output_data (&pbuf, &buf_size, current_config.addr_length,
cfi_info->start_address);
@@ -648,9 +749,6 @@ dot_cfi_endproc (void)
buf_size = 4;
output_data (&pbuf, &buf_size, t_long, fde_size - 4);
- /* Adjust initloc offset. */
- fde_initloc_offset += frag_now_fix ();
-
/* Copy FDE to objfile. */
where = (unsigned char *) frag_more (fde_size);
memcpy (where, fde_buf, fde_size);
@@ -691,6 +789,7 @@ dot_cfi (int arg)
case CFA_def_cfa_register:
case CFA_def_cfa_offset:
case CFA_offset:
+ case CFA_register:
case CFI_adjust_cfa_offset:
cfi_make_insn (arg);
break;
Index: testsuite/gas/cfi/cfi-i386.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/cfi/cfi-i386.d,v
retrieving revision 1.1
diff -u -p -r1.1 cfi-i386.d
--- testsuite/gas/cfi/cfi-i386.d 20 May 2003 08:01:19 -0000 1.1
+++ testsuite/gas/cfi/cfi-i386.d 20 May 2003 13:44:34 -0000
@@ -1,6 +1,5 @@
#readelf: -wf
#name: CFI on i386
-
The section .eh_frame contains:
00000000 00000010 00000000 CIE
@@ -22,19 +21,7 @@ The section .eh_frame contains:
DW_CFA_def_cfa_offset: 4
DW_CFA_nop
-0000002c 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-00000040 00000018 00000018 FDE cie=0000002c pc=00000012..0000001f
+0000002c 00000018 00000030 FDE cie=00000000 pc=00000012..0000001f
DW_CFA_advance_loc: 1 to 00000013
DW_CFA_def_cfa_offset: 8
DW_CFA_offset: r6 at cfa-8
@@ -44,56 +31,20 @@ The section .eh_frame contains:
DW_CFA_def_cfa_reg: r7
DW_CFA_nop
-0000005c 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-00000070 00000014 00000018 FDE cie=0000005c pc=0000001f..0000002f
+00000048 00000014 0000004c FDE cie=00000000 pc=0000001f..0000002f
DW_CFA_advance_loc: 2 to 00000021
DW_CFA_def_cfa_reg: r1
DW_CFA_advance_loc: 13 to 0000002e
DW_CFA_def_cfa: r7 ofs 4
DW_CFA_nop
-00000088 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-0000009c 00000010 00000018 FDE cie=00000088 pc=0000002f..00000035
+00000060 00000010 00000064 FDE cie=00000000 pc=0000002f..00000035
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
-000000b0 00000010 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -4
- Return address column: 8
-
- DW_CFA_def_cfa: r7 ofs 4
- DW_CFA_offset: r8 at cfa-4
- DW_CFA_nop
- DW_CFA_nop
-
-000000c4 00000010 00000018 FDE cie=000000b0 pc=00000035..00000044
+00000074 00000010 00000078 FDE cie=00000000 pc=00000035..00000044
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
Index: testsuite/gas/cfi/cfi-x86_64.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/cfi/cfi-x86_64.d,v
retrieving revision 1.1
diff -u -p -r1.1 cfi-x86_64.d
--- testsuite/gas/cfi/cfi-x86_64.d 20 May 2003 08:01:19 -0000 1.1
+++ testsuite/gas/cfi/cfi-x86_64.d 20 May 2003 13:44:34 -0000
@@ -26,23 +26,7 @@ The section .eh_frame contains:
DW_CFA_def_cfa_offset: 8
DW_CFA_nop
-00000038 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-00000050 00000024 0000001c FDE cie=00000038 pc=00000000..0000000f
+00000038 00000024 0000003c FDE cie=00000000 pc=00000000..0000000f
DW_CFA_advance_loc: 1 to 00000001
DW_CFA_def_cfa_offset: 16
DW_CFA_offset: r6 at cfa-16
@@ -55,23 +39,7 @@ The section .eh_frame contains:
DW_CFA_nop
DW_CFA_nop
-00000078 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-00000090 0000001c 0000001c FDE cie=00000078 pc=00000000..00000013
+00000060 0000001c 00000064 FDE cie=00000000 pc=00000000..00000013
DW_CFA_advance_loc: 3 to 00000003
DW_CFA_def_cfa_reg: r12
DW_CFA_advance_loc: 15 to 00000012
@@ -79,23 +47,7 @@ The section .eh_frame contains:
DW_CFA_nop
DW_CFA_nop
-000000b0 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-000000c8 0000001c 0000001c FDE cie=000000b0 pc=00000000..00000006
+00000080 0000001c 00000084 FDE cie=00000000 pc=00000000..00000006
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
@@ -105,23 +57,7 @@ The section .eh_frame contains:
DW_CFA_nop
DW_CFA_nop
-000000e8 00000014 00000000 CIE
- Version: 1
- Augmentation: ""
- Code alignment factor: 1
- Data alignment factor: -8
- Return address column: 16
-
- DW_CFA_def_cfa: r7 ofs 8
- DW_CFA_offset: r16 at cfa-8
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
- DW_CFA_nop
-
-00000100 0000001c 0000001c FDE cie=000000e8 pc=00000000..00000012
+000000a0 0000001c 000000a4 FDE cie=00000000 pc=00000000..00000012
DW_CFA_nop
DW_CFA_nop
DW_CFA_nop
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 13:55 ` Michal Ludvig
@ 2003-05-20 14:18 ` Alan Modra
2003-05-20 14:36 ` Michal Ludvig
0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2003-05-20 14:18 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils
On Tue, May 20, 2003 at 03:55:04PM +0200, Michal Ludvig wrote:
> 'indent' is still reverting this... Anyway I manually kept it your way
> in the diff.
I used emacs c-indent-exp and similar macros.
> >>+ if (cie_ptr->size == cie_size - 4 &&
> >>+ memcmp (cie_ptr->data, cie_buf + 4, cie_ptr->size) == 0)
> >
> >Again.
>
> Fixed.
You missed this one.
> OK to commit?
Yes, with that trailing operator fixed.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 14:18 ` Alan Modra
@ 2003-05-20 14:36 ` Michal Ludvig
0 siblings, 0 replies; 15+ messages in thread
From: Michal Ludvig @ 2003-05-20 14:36 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
Alan Modra told me that:
>>OK to commit?
>
> Yes, with that trailing operator fixed.
Thanks. Fixed and committed.
Michal Ludvig
--
* SuSE CR, s.r.o * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 12:21 [RFA] CFI improvements Michal Ludvig
2003-05-20 12:55 ` Andreas Jaeger
2003-05-20 12:57 ` Alan Modra
@ 2003-05-20 23:54 ` Hans-Peter Nilsson
2003-05-21 13:18 ` Michal Ludvig
2 siblings, 1 reply; 15+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-20 23:54 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils
On Tue, 20 May 2003, Michal Ludvig wrote:
> the attached patch adds some new features to the CFI engine:
> - register names can start with '%' as requested by GCC guys.
This should be handled by the target's tc_regname_to_dw2regnum,
not in target-independent code. (No creeping ix86-isms, please.)
brgds, H-P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-20 23:54 ` Hans-Peter Nilsson
@ 2003-05-21 13:18 ` Michal Ludvig
2003-05-21 14:35 ` Hans-Peter Nilsson
2003-05-21 15:06 ` Hans-Peter Nilsson
0 siblings, 2 replies; 15+ messages in thread
From: Michal Ludvig @ 2003-05-21 13:18 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 498 bytes --]
Hans-Peter Nilsson told me that:
> On Tue, 20 May 2003, Michal Ludvig wrote:
>
>>the attached patch adds some new features to the CFI engine:
>>- register names can start with '%' as requested by GCC guys.
>
> This should be handled by the target's tc_regname_to_dw2regnum,
> not in target-independent code. (No creeping ix86-isms, please.)
No problem. Something like this? Should I commit it?
Michal Ludvig
--
* SuSE CR, s.r.o * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz
[-- Attachment #2: gas-cfi-enh1-1.diff --]
[-- Type: text/plain, Size: 2734 bytes --]
2003-05-21 Michal Ludvig <mludvig@suse.cz>
* dw2gencfi.c (cfi_parse_arg): Allow registers in more
generic way. Remove special case for '%'-starting regnames.
* config/tc-i386.c (tc_x86_regname_to_dw2regnum): Allow
'%'-starting regnames.
* testsuite/gas/cfi/cfi-x86_64.s: Test register names
starting with '%'.
Index: dw2gencfi.c
===================================================================
RCS file: /cvs/src/src/gas/dw2gencfi.c,v
retrieving revision 1.4
diff -u -p -r1.4 dw2gencfi.c
--- dw2gencfi.c 21 May 2003 11:31:07 -0000 1.4
+++ dw2gencfi.c 21 May 2003 13:13:53 -0000
@@ -151,20 +151,24 @@ cfi_parse_arg (long *param, int resolver
retval = 1;
}
#ifdef tc_regname_to_dw2regnum
- else if (resolvereg && ((is_name_beginner (*input_line_pointer))
- || (*input_line_pointer == '%'
- && is_name_beginner (*(++input_line_pointer)))))
+ else if (resolvereg)
{
char *name, c, *p;
name = input_line_pointer;
- c = get_symbol_end ();
- p = input_line_pointer;
+ p = name;
+ /* Recognize end of argument. */
+ while (*p != ',' && *p != ';' && *p != ' '
+ && *p != '\t' && *p != '\n' && *p != '\r')
+ p++;
+ c = *p;
+ *p = 0;
if ((value = tc_regname_to_dw2regnum (name)) >= 0)
retval = 1;
*p = c;
+ input_line_pointer = p;
}
#endif
else
Index: config/tc-i386.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-i386.c,v
retrieving revision 1.139
diff -u -p -r1.139 tc-i386.c
--- config/tc-i386.c 20 May 2003 07:58:06 -0000 1.139
+++ config/tc-i386.c 21 May 2003 13:13:54 -0000
@@ -6360,6 +6360,9 @@ tc_x86_regname_to_dw2regnum (const char
regnames_count = sizeof (regnames_32);
}
+ if (*regname == '%')
+ regname ++;
+
for (regnum = 0; regnum < regnames_count; regnum++)
if (strcmp (regname, regnames[regnum]) == 0)
return regnum;
Index: testsuite/gas/cfi/cfi-x86_64.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/cfi/cfi-x86_64.s,v
retrieving revision 1.1
diff -u -p -r1.1 cfi-x86_64.s
--- testsuite/gas/cfi/cfi-x86_64.s 20 May 2003 08:01:19 -0000 1.1
+++ testsuite/gas/cfi/cfi-x86_64.s 21 May 2003 13:13:54 -0000
@@ -35,9 +35,9 @@ func_prologue:
#; each instruction.
pushq %rbp
.cfi_def_cfa_offset 16
- .cfi_offset rbp,-16
+ .cfi_offset %rbp, -16
movq %rsp, %rbp
- .cfi_def_cfa_register rbp
+ .cfi_def_cfa_register %rbp
#; function body
call func_locvars
@@ -46,7 +46,7 @@ func_prologue:
#; epilogue with valid CFI
#; (we're better than gcc :-)
leaveq
- .cfi_def_cfa rsp,8
+ .cfi_def_cfa %rsp, 8
ret
.cfi_endproc
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-21 13:18 ` Michal Ludvig
@ 2003-05-21 14:35 ` Hans-Peter Nilsson
2003-05-21 15:06 ` Hans-Peter Nilsson
1 sibling, 0 replies; 15+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-21 14:35 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils
On Wed, 21 May 2003, Michal Ludvig wrote:
> Hans-Peter Nilsson told me that:
> > On Tue, 20 May 2003, Michal Ludvig wrote:
> >
> >>the attached patch adds some new features to the CFI engine:
> >>- register names can start with '%' as requested by GCC guys.
> >
> > This should be handled by the target's tc_regname_to_dw2regnum,
> > not in target-independent code. (No creeping ix86-isms, please.)
>
> No problem. Something like this?
Fine by me, but drop the space in "regname ++".
> Should I commit it?
(I can't say.)
brgds, H-P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-21 13:18 ` Michal Ludvig
2003-05-21 14:35 ` Hans-Peter Nilsson
@ 2003-05-21 15:06 ` Hans-Peter Nilsson
2003-05-21 16:44 ` Michal Ludvig
1 sibling, 1 reply; 15+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-21 15:06 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils
On Wed, 21 May 2003, Michal Ludvig wrote:
> Hans-Peter Nilsson told me that:
> > On Tue, 20 May 2003, Michal Ludvig wrote:
> >
> >>the attached patch adds some new features to the CFI engine:
> >>- register names can start with '%' as requested by GCC guys.
> >
> > This should be handled by the target's tc_regname_to_dw2regnum,
> > not in target-independent code. (No creeping ix86-isms, please.)
>
> No problem. Something like this?
Oh, sorry: consider redefining tc_regname_to_dw2regnum to take a
char ** and to handle incrementing it over the register name.
Then you won't have to make assumptions on the syntax of the
regname in cfi_parse_arg.
The testcase augmentation you sent seem obvious.
brgds, H-P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-21 15:06 ` Hans-Peter Nilsson
@ 2003-05-21 16:44 ` Michal Ludvig
2003-05-21 16:52 ` Hans-Peter Nilsson
2003-05-27 12:34 ` amodra
0 siblings, 2 replies; 15+ messages in thread
From: Michal Ludvig @ 2003-05-21 16:44 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]
Hans-Peter Nilsson told me that:
> On Wed, 21 May 2003, Michal Ludvig wrote:
>
>>Hans-Peter Nilsson told me that:
>>
>>>On Tue, 20 May 2003, Michal Ludvig wrote:
>>>
>>>>the attached patch adds some new features to the CFI engine:
>>>>- register names can start with '%' as requested by GCC guys.
>>>
>>>This should be handled by the target's tc_regname_to_dw2regnum,
>>>not in target-independent code. (No creeping ix86-isms, please.)
>>
>>No problem. Something like this?
>
> Oh, sorry: consider redefining tc_regname_to_dw2regnum to take a
> char ** and to handle incrementing it over the register name.
It's not good a idea because you couldn't pass constant strings to the
function then. Instead I added another parameter - pointer to 'int' -
that is filled with the number of characters used. If the 'size' is NULL
nothing bad happens - the value is simply ignored. After return size can
be used to adjust input_line_pointer.
Comments? Can I commit this version (Alan)?
Michal Ludvig
--
* SuSE CR, s.r.o * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz
[-- Attachment #2: gas-cfi-enh1-3.diff --]
[-- Type: text/plain, Size: 6743 bytes --]
2003-05-21 Michal Ludvig <mludvig@suse.cz>
* dw2gencfi.c (cfi_parse_arg): Allow registers in more
generic way. Remove special case for '%'-starting regnames.
* config/tc-i386.c (tc_x86_regname_to_dw2regnum): Allow
'%'-starting regnames.
* testsuite/gas/cfi/cfi-x86_64.s: Test register names
starting with '%' and shorter than 3 chars.
Index: dw2gencfi.c
===================================================================
RCS file: /cvs/src/src/gas/dw2gencfi.c,v
retrieving revision 1.4
diff -u -p -r1.4 dw2gencfi.c
--- dw2gencfi.c 21 May 2003 11:31:07 -0000 1.4
+++ dw2gencfi.c 21 May 2003 16:32:37 -0000
@@ -151,20 +151,15 @@ cfi_parse_arg (long *param, int resolver
retval = 1;
}
#ifdef tc_regname_to_dw2regnum
- else if (resolvereg && ((is_name_beginner (*input_line_pointer))
- || (*input_line_pointer == '%'
- && is_name_beginner (*(++input_line_pointer)))))
+ else if (resolvereg)
{
- char *name, c, *p;
-
- name = input_line_pointer;
- c = get_symbol_end ();
- p = input_line_pointer;
-
- if ((value = tc_regname_to_dw2regnum (name)) >= 0)
+ int size;
+
+ if ((value = tc_regname_to_dw2regnum (input_line_pointer, &size)) >= 0)
+ {
retval = 1;
-
- *p = c;
+ input_line_pointer += size;
+ }
}
#endif
else
Index: config/tc-i386.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-i386.c,v
retrieving revision 1.139
diff -u -p -r1.139 tc-i386.c
--- config/tc-i386.c 20 May 2003 07:58:06 -0000 1.139
+++ config/tc-i386.c 21 May 2003 16:32:38 -0000
@@ -6329,10 +6329,11 @@ tc_x86_cfi_init (void)
}
unsigned long
-tc_x86_regname_to_dw2regnum (const char *regname)
+tc_x86_regname_to_dw2regnum (char *regname, int *size)
{
unsigned int regnum;
unsigned int regnames_count;
+ unsigned int percent_flag = 0, name_len;
char *regnames_32[] =
{
"eax", "ebx", "ecx", "edx",
@@ -6347,24 +6348,52 @@ tc_x86_regname_to_dw2regnum (const char
"r12", "r13", "r14", "r15",
"rip"
};
- char **regnames;
+ char **regnames, *p, c;
if (flag_code == CODE_64BIT)
{
regnames = regnames_64;
- regnames_count = sizeof (regnames_64);
+ regnames_count = sizeof (regnames_64) / sizeof (void*);
}
else
{
regnames = regnames_32;
- regnames_count = sizeof (regnames_32);
+ regnames_count = sizeof (regnames_32) / sizeof (void*);
}
+ percent_flag = (*regname == '%');
+ regname += percent_flag;
+
+ p = regname;
+ while ((*p >= 'a' && *p <= 'z')
+ || (*p >= 'A' && *p <= 'Z')
+ || (*p >= '0' && *p <= '9'))
+ p++;
+
+ /* Save char at *p only when it isn't \0. */
+ if (*p)
+ {
+ c = *p;
+ *p = 0;
+ }
+
+ name_len = p - regname;
+
for (regnum = 0; regnum < regnames_count; regnum++)
if (strcmp (regname, regnames[regnum]) == 0)
+ {
+ if (size)
+ *size = percent_flag + name_len;
+ if (c)
+ *p = c;
return regnum;
+ }
as_bad (_("unknown register name '%s'"), regname);
+ if (size)
+ *size = percent_flag + name_len;
+ if (c)
+ *p = c;
return -1;
}
@@ -6373,12 +6402,12 @@ tc_x86_frame_initial_instructions (void)
{
if (flag_code == CODE_64BIT)
{
- cfi_add_insn (CFA_def_cfa, tc_x86_regname_to_dw2regnum ("rsp"), 8);
- cfi_add_insn (CFA_offset, tc_x86_regname_to_dw2regnum ("rip"), -8);
+ cfi_add_insn (CFA_def_cfa, tc_x86_regname_to_dw2regnum ("rsp", NULL), 8);
+ cfi_add_insn (CFA_offset, tc_x86_regname_to_dw2regnum ("rip", NULL), -8);
}
else
{
- cfi_add_insn (CFA_def_cfa, tc_x86_regname_to_dw2regnum ("esp"), 4);
- cfi_add_insn (CFA_offset, tc_x86_regname_to_dw2regnum ("eip"), -4);
+ cfi_add_insn (CFA_def_cfa, tc_x86_regname_to_dw2regnum ("esp", NULL), 4);
+ cfi_add_insn (CFA_offset, tc_x86_regname_to_dw2regnum ("eip", NULL), -4);
}
}
Index: config/tc-i386.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-i386.h,v
retrieving revision 1.40
diff -u -p -r1.40 tc-i386.h
--- config/tc-i386.h 20 May 2003 07:58:07 -0000 1.40
+++ config/tc-i386.h 21 May 2003 16:32:38 -0000
@@ -553,7 +553,8 @@ extern void sco_id PARAMS ((void));
extern void tc_x86_cfi_init PARAMS ((void));
#define tc_regname_to_dw2regnum tc_x86_regname_to_dw2regnum
-extern unsigned long tc_x86_regname_to_dw2regnum PARAMS ((const char *regname));
+extern unsigned long tc_x86_regname_to_dw2regnum PARAMS ((char *regname,
+ int *size));
#define tc_cfi_frame_initial_instructions tc_x86_frame_initial_instructions
extern void tc_x86_frame_initial_instructions PARAMS ((void));
Index: testsuite/gas/cfi/cfi-x86_64.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/cfi/cfi-x86_64.d,v
retrieving revision 1.2
diff -u -p -r1.2 cfi-x86_64.d
--- testsuite/gas/cfi/cfi-x86_64.d 20 May 2003 14:31:44 -0000 1.2
+++ testsuite/gas/cfi/cfi-x86_64.d 21 May 2003 16:32:38 -0000
@@ -41,7 +41,7 @@ The section .eh_frame contains:
00000060 0000001c 00000064 FDE cie=00000000 pc=00000000..00000013
DW_CFA_advance_loc: 3 to 00000003
- DW_CFA_def_cfa_reg: r12
+ DW_CFA_def_cfa_reg: r8
DW_CFA_advance_loc: 15 to 00000012
DW_CFA_def_cfa_reg: r7
DW_CFA_nop
Index: testsuite/gas/cfi/cfi-x86_64.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/cfi/cfi-x86_64.s,v
retrieving revision 1.1
diff -u -p -r1.1 cfi-x86_64.s
--- testsuite/gas/cfi/cfi-x86_64.s 20 May 2003 08:01:19 -0000 1.1
+++ testsuite/gas/cfi/cfi-x86_64.s 21 May 2003 16:32:38 -0000
@@ -35,9 +35,9 @@ func_prologue:
#; each instruction.
pushq %rbp
.cfi_def_cfa_offset 16
- .cfi_offset rbp,-16
+ .cfi_offset %rbp, -16
movq %rsp, %rbp
- .cfi_def_cfa_register rbp
+ .cfi_def_cfa_register %rbp
#; function body
call func_locvars
@@ -46,7 +46,7 @@ func_prologue:
#; epilogue with valid CFI
#; (we're better than gcc :-)
leaveq
- .cfi_def_cfa rsp,8
+ .cfi_def_cfa %rsp, 8
ret
.cfi_endproc
@@ -59,21 +59,21 @@ func_prologue:
func_otherreg:
.cfi_startproc
- #; save frame pointer to r12
- movq %rsp,%r12
- .cfi_def_cfa_register r12
+ #; save frame pointer to r8
+ movq %rsp,%r8
+ .cfi_def_cfa_register r8
#; alocate space for local vars
#; (no .cfi_{def,adjust}_cfa_offset here,
- #; because CFA is computed from r12!)
+ #; because CFA is computed from r8!)
sub $100,%rsp
#; function body
call func_prologue
addl $2, %eax
- #; restore frame pointer from r12
- movq %r12,%rsp
+ #; restore frame pointer from r8
+ movq %r8,%rsp
.cfi_def_cfa_register rsp
ret
.cfi_endproc
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-21 16:44 ` Michal Ludvig
@ 2003-05-21 16:52 ` Hans-Peter Nilsson
2003-05-27 12:34 ` amodra
1 sibling, 0 replies; 15+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-21 16:52 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils
On Wed, 21 May 2003, Michal Ludvig wrote:
> Hans-Peter Nilsson told me that:
> > Oh, sorry: consider redefining tc_regname_to_dw2regnum to take a
> > char ** and to handle incrementing it over the register name.
>
> It's not good a idea because you couldn't pass constant strings to the
> function then.
Oh, right.
> Instead I added another parameter - pointer to 'int' -
> that is filled with the number of characters used. If the 'size' is NULL
> nothing bad happens - the value is simply ignored. After return size can
> be used to adjust input_line_pointer.
>
> Comments?
Looks good to me. (But use two spaces after dot in ChangeLog too.)
Thanks for doing this.
brgds, H-P
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-21 16:44 ` Michal Ludvig
2003-05-21 16:52 ` Hans-Peter Nilsson
@ 2003-05-27 12:34 ` amodra
2003-05-27 12:42 ` Michal Ludvig
1 sibling, 1 reply; 15+ messages in thread
From: amodra @ 2003-05-27 12:34 UTC (permalink / raw)
To: Michal Ludvig; +Cc: binutils
On Wed, May 21, 2003 at 06:44:14PM +0200, Michal Ludvig wrote:
> + if ((value = tc_regname_to_dw2regnum (input_line_pointer, &size)) >= 0)
> + {
Here and other places you indent braces incorrectly. Please fix.
> - regnames_count = sizeof (regnames_64);
> + regnames_count = sizeof (regnames_64) / sizeof (void*);
Better to divide by sizeof (regnames_64[0]), then it's obvious in
just this one line of code that you're calculating an array size.
> - regnames_count = sizeof (regnames_32);
> + regnames_count = sizeof (regnames_32) / sizeof (void*);
Same thing here.
> + while ((*p >= 'a' && *p <= 'z')
> + || (*p >= 'A' && *p <= 'Z')
> + || (*p >= '0' && *p <= '9'))
while (ISALNUM (*p))
> + /* Save char at *p only when it isn't \0. */
Why not save it when it is 0 too? Saves a test, here and later when
you restore.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] CFI improvements
2003-05-27 12:34 ` amodra
@ 2003-05-27 12:42 ` Michal Ludvig
0 siblings, 0 replies; 15+ messages in thread
From: Michal Ludvig @ 2003-05-27 12:42 UTC (permalink / raw)
To: amodra; +Cc: binutils
amodra@bigpond.net.au told me that:
> On Wed, May 21, 2003 at 06:44:14PM +0200, Michal Ludvig wrote:
>
>>- regnames_count = sizeof (regnames_64);
>>+ regnames_count = sizeof (regnames_64) / sizeof (void*);
>
> Better to divide by sizeof (regnames_64[0]), then it's obvious in
> just this one line of code that you're calculating an array size.
OK.
>>+ while ((*p >= 'a' && *p <= 'z')
>>+ || (*p >= 'A' && *p <= 'Z')
>>+ || (*p >= '0' && *p <= '9'))
>
> while (ISALNUM (*p))
OK.
>>+ /* Save char at *p only when it isn't \0. */
>
> Why not save it when it is 0 too? Saves a test, here and later when
> you restore.
If I pass a register name in a constant pointer I expect it is
terminated by \0 and so I don't write to it to avoid segfault. But it's
a bit fragile, I know. I'll rewrite the function to make a local copy of
the string and work on it instead.
Michal Ludvig
--
* SuSE CR, s.r.o * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2003-05-27 12:42 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-05-20 12:21 [RFA] CFI improvements Michal Ludvig
2003-05-20 12:55 ` Andreas Jaeger
2003-05-20 12:57 ` Alan Modra
2003-05-20 13:10 ` Michal Ludvig
2003-05-20 13:55 ` Michal Ludvig
2003-05-20 14:18 ` Alan Modra
2003-05-20 14:36 ` Michal Ludvig
2003-05-20 23:54 ` Hans-Peter Nilsson
2003-05-21 13:18 ` Michal Ludvig
2003-05-21 14:35 ` Hans-Peter Nilsson
2003-05-21 15:06 ` Hans-Peter Nilsson
2003-05-21 16:44 ` Michal Ludvig
2003-05-21 16:52 ` Hans-Peter Nilsson
2003-05-27 12:34 ` amodra
2003-05-27 12:42 ` Michal Ludvig
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).