* demand_empty_rest_of_line and ignore_rest_of_line @ 2004-03-17 16:36 Nathan Sidwell 2004-03-17 16:43 ` Hans-Peter Nilsson 2004-03-17 16:49 ` Ian Lance Taylor 0 siblings, 2 replies; 41+ messages in thread From: Nathan Sidwell @ 2004-03-17 16:36 UTC (permalink / raw) To: binutils Hi, what is the intended difference between demand_empty_rest_of_line and ignore_rest_of_line? From the source I see that demand_empty_ROL skips whitespace but ignore_ROL does not. Then, demand_empty falls into ignore_ROL to issue a warning and skip up to the EOL. All but one use of ignore_ROL I saw (I've not checked the config dir yet) were of the form as_{bad,warn} ("something bad happened"); ignore_rest_of_line (); which implies to me that ignore_ROL should be silent. Also, what do people think about demand_empty_ROL issuing an error? IMHO, if the syntax requires no more stuff, it's an error if there is more stuff. Would a patch which made ignore_ROL silent and demand_empty_ROL issue an error be acceptable? nathan -- Nathan Sidwell :: http://www.codesourcery.com :: CodeSourcery LLC nathan@codesourcery.com :: http://www.planetfall.pwp.blueyonder.co.uk ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-03-17 16:36 demand_empty_rest_of_line and ignore_rest_of_line Nathan Sidwell @ 2004-03-17 16:43 ` Hans-Peter Nilsson 2004-03-17 16:49 ` Ian Lance Taylor 1 sibling, 0 replies; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-03-17 16:43 UTC (permalink / raw) To: Nathan Sidwell; +Cc: binutils On Wed, 17 Mar 2004, Nathan Sidwell wrote: > what is the intended difference between demand_empty_rest_of_line and > ignore_rest_of_line? From memory and trigged by the names (i.e. not cheating and looking at the code ;-) one is supposed to issue an error (demand_*) and the other (ignore_*) should just skip to the next line (and update whatever counters and such). If they don't, I'd argue it's confusing enough that a change is warranted. > which implies to me that ignore_ROL should be silent. It's not? > Also, what do people think about demand_empty_ROL issuing an error? It doesn't? :-) > IMHO, if the syntax requires no more stuff, it's an error if there is > more stuff. > > Would a patch which made ignore_ROL silent and demand_empty_ROL issue > an error be acceptable? It's used by config/tc-* so I suggest regression checking enough targets to cover all target uses of demand_* and ignore_*. brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-03-17 16:36 demand_empty_rest_of_line and ignore_rest_of_line Nathan Sidwell 2004-03-17 16:43 ` Hans-Peter Nilsson @ 2004-03-17 16:49 ` Ian Lance Taylor 2004-03-18 10:29 ` Nathan Sidwell 1 sibling, 1 reply; 41+ messages in thread From: Ian Lance Taylor @ 2004-03-17 16:49 UTC (permalink / raw) To: Nathan Sidwell; +Cc: binutils Nathan Sidwell <nathan@codesourcery.com> writes: > what is the intended difference between demand_empty_rest_of_line and > ignore_rest_of_line? From the source I see that demand_empty_ROL skips > whitespace but ignore_ROL does not. Then, demand_empty falls into > ignore_ROL to issue a warning and skip up to the EOL. All but one use > of ignore_ROL I saw (I've not checked the config dir yet) were of the form > as_{bad,warn} ("something bad happened"); > ignore_rest_of_line (); > which implies to me that ignore_ROL should be silent. demand_empty_rest_of_line is called when the rest of the line is required to be empty, and we have no reason to think that it is not empty. ignore_rest_of_line is called after an error occurs, and is intended to help the user see where the error happened, by indicating what point the assembler reached on the line before it got the error. So, I agree that demand_empty_rest_of_line should probably issue an error rather than a warning. I'm agnostic about whether ignore_rest_of_line should be silent. The current code is not a mistake. But it's fine with me to change it if making it shut up appears more useful in practice. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-03-17 16:49 ` Ian Lance Taylor @ 2004-03-18 10:29 ` Nathan Sidwell 2004-03-18 13:15 ` Ian Lance Taylor 2004-04-23 23:15 ` Andreas Schwab 0 siblings, 2 replies; 41+ messages in thread From: Nathan Sidwell @ 2004-03-18 10:29 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: binutils [-- Attachment #1: Type: text/plain, Size: 562 bytes --] Hi, This patch implements *) demand_empty_rest_of_line issues an error and skips to EOL *) ignore_rest_of_line silently skips to EOL. I went through the config dir and inspected all uses of ignore_rest_of_line. Those that needed to be demand_empty_rest_of_line are changed. I tested with the following targets, arc-unknown-elf i686-pc-linux-gnu ia64-unknown-linux-gnu mips-unknown-elf ok? nathan -- Nathan Sidwell :: http://www.codesourcery.com :: CodeSourcery LLC nathan@codesourcery.com :: http://www.planetfall.pwp.blueyonder.co.uk [-- Attachment #2: EOL.patch --] [-- Type: text/plain, Size: 8632 bytes --] 2004-03-18 Nathan Sidwell <nathan@codesourcery.com> * read.c (read_a_source_file): Use demand_empty_rest_of_line. (demand_empty_rest_of_line): Issue an error here. (ignore_rest_of_line): Silently skip to end. (demand_copy_string): Issue an error, not warning. (equals): Likewise. * config/obj-elf.c (obj_elf_section_name): Likewise. (obj_elf_section): Likewise. * config/tc-arc.c (arc_extoper): Remove bogus NULL checks. (arc_extinst): Likewise. * config/tc-ia64.c (dot_saveb): Use demand_empty_rest_of_line. (dot_spill): Likewise. (dot_unwabi): Likewise. (dot_prologue): Likewise. Index: read.c =================================================================== RCS file: /cvs/src/src/gas/read.c,v retrieving revision 1.76 diff -c -3 -p -r1.76 read.c *** read.c 12 Mar 2004 17:48:12 -0000 1.76 --- read.c 18 Mar 2004 09:56:05 -0000 *************** read_a_source_file (char *name) *** 1053,1059 **** #endif input_line_pointer--; /* Report unknown char as ignored. */ ! ignore_rest_of_line (); } #ifdef md_after_pass_hook --- 1053,1059 ---- #endif input_line_pointer--; /* Report unknown char as ignored. */ ! demand_empty_rest_of_line (); } #ifdef md_after_pass_hook *************** s_text (int ignore ATTRIBUTE_UNUSED) *** 3020,3025 **** --- 3020,3029 ---- #endif } \f + + /* Verify that we are at the end of a line. If not, issue an error and + skip to EOL. */ + void demand_empty_rest_of_line (void) { *************** demand_empty_rest_of_line (void) *** 3027,3054 **** if (is_end_of_line[(unsigned char) *input_line_pointer]) input_line_pointer++; else - ignore_rest_of_line (); - - /* Return having already swallowed end-of-line. */ - } - - void - ignore_rest_of_line (void) - { - /* For suspect lines: gives warning. */ - if (!is_end_of_line[(unsigned char) *input_line_pointer]) { if (ISPRINT (*input_line_pointer)) ! as_warn (_("rest of line ignored; first ignored character is `%c'"), *input_line_pointer); else ! as_warn (_("rest of line ignored; first ignored character valued 0x%x"), *input_line_pointer); ! ! while (input_line_pointer < buffer_limit ! && !is_end_of_line[(unsigned char) *input_line_pointer]) ! input_line_pointer++; } input_line_pointer++; --- 3031,3059 ---- if (is_end_of_line[(unsigned char) *input_line_pointer]) input_line_pointer++; else { if (ISPRINT (*input_line_pointer)) ! as_bad (_("junk at end of line, first unrecognized character is `%c'"), *input_line_pointer); else ! as_bad (_("junk at end of line, first unrecognized character valued 0x%x"), *input_line_pointer); ! ignore_rest_of_line (); } + + /* Return pointing just after end-of-line. */ + know (is_end_of_line[(unsigned char) input_line_pointer[-1]]); + } + + /* Silently advance to the end of line. Use this after already having + issued an error about something bad. */ + + void + ignore_rest_of_line (void) + { + while (input_line_pointer < buffer_limit + && !is_end_of_line[(unsigned char) *input_line_pointer]) + input_line_pointer++; input_line_pointer++; *************** demand_copy_string (int *lenP) *** 4738,4744 **** } else { ! as_warn (_("missing string")); retval = NULL; ignore_rest_of_line (); } --- 4743,4749 ---- } else { ! as_bad (_("missing string")); retval = NULL; ignore_rest_of_line (); } *************** equals (char *sym_name, int reassign) *** 4814,4820 **** if (flag_mri) { /* Check garbage after the expression. */ ! ignore_rest_of_line (); mri_comment_end (stop, stopc); } } --- 4819,4825 ---- if (flag_mri) { /* Check garbage after the expression. */ ! demand_empty_rest_of_line (); mri_comment_end (stop, stopc); } } Index: config/obj-elf.c =================================================================== RCS file: /cvs/src/src/gas/config/obj-elf.c,v retrieving revision 1.79 diff -c -3 -p -r1.79 obj-elf.c *** config/obj-elf.c 13 Dec 2003 12:57:40 -0000 1.79 --- config/obj-elf.c 18 Mar 2004 09:56:09 -0000 *************** obj_elf_section_name (void) *** 787,793 **** end++; if (end == input_line_pointer) { ! as_warn (_("missing name")); ignore_rest_of_line (); return NULL; } --- 787,793 ---- end++; if (end == input_line_pointer) { ! as_bad (_("missing name")); ignore_rest_of_line (); return NULL; } *************** obj_elf_section (int push) *** 938,944 **** SKIP_WHITESPACE (); if (*input_line_pointer != '#') { ! as_warn (_("character following name is not '#'")); ignore_rest_of_line (); return; } --- 938,944 ---- SKIP_WHITESPACE (); if (*input_line_pointer != '#') { ! as_bad (_("character following name is not '#'")); ignore_rest_of_line (); return; } Index: config/tc-arc.c =================================================================== RCS file: /cvs/src/src/gas/config/tc-arc.c,v retrieving revision 1.25 diff -c -3 -p -r1.25 tc-arc.c *** config/tc-arc.c 10 Dec 2003 06:41:08 -0000 1.25 --- config/tc-arc.c 18 Mar 2004 09:56:13 -0000 *************** arc_extoper (opertype) *** 905,915 **** name = input_line_pointer; c = get_symbol_end (); name = xstrdup (name); - if (NULL == name) - { - ignore_rest_of_line (); - return; - } p = name; while (*p) --- 905,910 ---- *************** arc_extinst (ignore) *** 1153,1163 **** name = input_line_pointer; c = get_symbol_end (); name = xstrdup (name); - if (NULL == name) - { - ignore_rest_of_line (); - return; - } strcpy (syntax, name); name_len = strlen (name); --- 1148,1153 ---- *************** arc_extinst (ignore) *** 1305,1322 **** strcat (syntax, "%S%L"); ext_op = (struct arc_opcode *) xmalloc (sizeof (struct arc_opcode)); - if (NULL == ext_op) - { - ignore_rest_of_line (); - return; - } - ext_op->syntax = xstrdup (syntax); - if (NULL == ext_op->syntax) - { - ignore_rest_of_line (); - return; - } ext_op->mask = I (-1) | ((0x3 == opcode) ? C (-1) : 0); ext_op->value = I (opcode) | ((0x3 == opcode) ? C (subopcode) : 0); --- 1295,1301 ---- Index: config/tc-ia64.c =================================================================== RCS file: /cvs/src/src/gas/config/tc-ia64.c,v retrieving revision 1.106 diff -c -3 -p -r1.106 tc-ia64.c *** config/tc-ia64.c 5 Mar 2004 17:07:12 -0000 1.106 --- config/tc-ia64.c 18 Mar 2004 09:56:32 -0000 *************** dot_saveb (dummy) *** 3617,3623 **** add_unwind_entry (output_br_mem (brmask)); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! ignore_rest_of_line (); } static void --- 3617,3623 ---- add_unwind_entry (output_br_mem (brmask)); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! demand_empty_rest_of_line (); } static void *************** dot_spill (dummy) *** 3649,3655 **** sep = parse_operand (&e); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! ignore_rest_of_line (); if (e.X_op != O_constant) as_bad ("Operand to .spill must be a constant"); --- 3649,3655 ---- sep = parse_operand (&e); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! demand_empty_rest_of_line (); if (e.X_op != O_constant) as_bad ("Operand to .spill must be a constant"); *************** dot_unwabi (dummy) *** 3925,3931 **** } sep = parse_operand (&e2); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! ignore_rest_of_line (); if (e1.X_op != O_constant) { --- 3925,3931 ---- } sep = parse_operand (&e2); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! demand_empty_rest_of_line (); if (e1.X_op != O_constant) { *************** dot_prologue (dummy) *** 4020,4026 **** as_bad ("No second operand to .prologue"); sep = parse_operand (&e2); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! ignore_rest_of_line (); if (e1.X_op == O_constant) { --- 4020,4026 ---- as_bad ("No second operand to .prologue"); sep = parse_operand (&e2); if (!is_end_of_line[sep] && !is_it_end_of_statement ()) ! demand_empty_rest_of_line (); if (e1.X_op == O_constant) { ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-03-18 10:29 ` Nathan Sidwell @ 2004-03-18 13:15 ` Ian Lance Taylor 2004-04-23 23:15 ` Andreas Schwab 1 sibling, 0 replies; 41+ messages in thread From: Ian Lance Taylor @ 2004-03-18 13:15 UTC (permalink / raw) To: Nathan Sidwell; +Cc: binutils Nathan Sidwell <nathan@codesourcery.com> writes: > 2004-03-18 Nathan Sidwell <nathan@codesourcery.com> > > * read.c (read_a_source_file): Use demand_empty_rest_of_line. > (demand_empty_rest_of_line): Issue an error here. > (ignore_rest_of_line): Silently skip to end. > (demand_copy_string): Issue an error, not warning. > (equals): Likewise. > * config/obj-elf.c (obj_elf_section_name): Likewise. > (obj_elf_section): Likewise. > * config/tc-arc.c (arc_extoper): Remove bogus NULL checks. > (arc_extinst): Likewise. > * config/tc-ia64.c (dot_saveb): Use demand_empty_rest_of_line. > (dot_spill): Likewise. > (dot_unwabi): Likewise. > (dot_prologue): Likewise. OK. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-03-18 10:29 ` Nathan Sidwell 2004-03-18 13:15 ` Ian Lance Taylor @ 2004-04-23 23:15 ` Andreas Schwab 2004-04-24 17:36 ` Hans-Peter Nilsson 1 sibling, 1 reply; 41+ messages in thread From: Andreas Schwab @ 2004-04-23 23:15 UTC (permalink / raw) To: Nathan Sidwell; +Cc: Ian Lance Taylor, binutils Nathan Sidwell <nathan@codesourcery.com> writes: > Index: read.c > =================================================================== > RCS file: /cvs/src/src/gas/read.c,v > retrieving revision 1.76 > diff -c -3 -p -r1.76 read.c > *** read.c 12 Mar 2004 17:48:12 -0000 1.76 > --- read.c 18 Mar 2004 09:56:05 -0000 > *************** read_a_source_file (char *name) > *** 1053,1059 **** > #endif > input_line_pointer--; > /* Report unknown char as ignored. */ > ! ignore_rest_of_line (); > } > > #ifdef md_after_pass_hook > --- 1053,1059 ---- > #endif > input_line_pointer--; > /* Report unknown char as ignored. */ > ! demand_empty_rest_of_line (); > } > > #ifdef md_after_pass_hook This means that the unknown character is no longer ignored, despite the comment. As a side effect a line starting with a line comment character not followed by APP in NO_APP mode now triggers an error instead of just a warning, breaking builds of glibc on m68k-linux. Earlier in read_a_source_file where #APP is handled there is another comment that claims that unknown comments are ignored, when in fact they aren't (only the initial line comment character is skipped). Note that the presence of #APP will mess up the line counters, but that appears to be difficult to fix. Andreas. 2004-04-23 Andreas Schwab <schwab@suse.de> * read.c (read_a_source_file): Ignore unknown text after line comment character. Fix misleading comment. --- gas/read.c.~1.78.~ 2004-04-23 08:58:23.000000000 +0200 +++ gas/read.c 2004-04-23 21:49:01.000000000 +0200 @@ -1,6 +1,6 @@ /* read.c - read a source file - Copyright 1986, 1987, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, - 1998, 1999, 2000, 2001, 2002, 2003 Free Software Foundation, Inc. + 1998, 1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc. This file is part of GAS, the GNU Assembler. @@ -950,10 +950,14 @@ read_a_source_file (char *name) unsigned int new_length; char *tmp_buf = 0; - bump_line_counters (); s = input_line_pointer; if (strncmp (s, "APP\n", 4)) - continue; /* We ignore it */ + { + /* We ignore it */ + ignore_rest_of_line (); + continue; + } + bump_line_counters (); s += 4; sb_new (&sbuf); @@ -1052,7 +1056,7 @@ read_a_source_file (char *name) continue; #endif input_line_pointer--; - /* Report unknown char as ignored. */ + /* Report unknown char as error. */ demand_empty_rest_of_line (); } -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-23 23:15 ` Andreas Schwab @ 2004-04-24 17:36 ` Hans-Peter Nilsson 2004-04-24 17:57 ` Andreas Schwab 0 siblings, 1 reply; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-24 17:36 UTC (permalink / raw) To: Andreas Schwab; +Cc: Nathan Sidwell, Ian Lance Taylor, binutils On Fri, 23 Apr 2004, Andreas Schwab wrote: > This means that the unknown character is no longer ignored, despite the > comment. As a side effect a line starting with a line comment character > not followed by APP in NO_APP mode now triggers an error instead of just a > warning, breaking builds of glibc on m68k-linux. For the record, there aren't supposed to *be* any comments in #NO_APP mode; just strict formatted assembly code and #APP of course. That's what it means. If the glibc __sec_comment kludge is the problem, let me mention that different patches have been sent to glibc in order to make its kludge work properly(!) at least one that adds #NO_APP and I'm sure would work, but it seems none have been applied. Of course, we could do something that lets glibc emit its .gnu.warning.* sections without assembler warnings, and avoid using the __sec_comment kludge where the new thing is supported. brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 17:36 ` Hans-Peter Nilsson @ 2004-04-24 17:57 ` Andreas Schwab 2004-04-24 18:26 ` Hans-Peter Nilsson 0 siblings, 1 reply; 41+ messages in thread From: Andreas Schwab @ 2004-04-24 17:57 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: Nathan Sidwell, Ian Lance Taylor, binutils Hans-Peter Nilsson <hp@bitrange.com> writes: > If the glibc __sec_comment kludge is the problem, let me mention > that different patches have been sent to glibc in order to make > its kludge work properly(!) at least one that adds #NO_APP and > I'm sure would work, but it seems none have been applied. Unfortunately, m68k and arm are the only gcc targets supported by glibc that defaults to #NO_APP, all other targets don't have this problem. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux AG, MaxfeldstraÃe 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 17:57 ` Andreas Schwab @ 2004-04-24 18:26 ` Hans-Peter Nilsson 2004-04-24 19:22 ` Andreas Schwab 0 siblings, 1 reply; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-24 18:26 UTC (permalink / raw) To: Andreas Schwab; +Cc: Nathan Sidwell, Ian Lance Taylor, binutils On Sat, 24 Apr 2004, Andreas Schwab wrote: > Hans-Peter Nilsson <hp@bitrange.com> writes: > > > If the glibc __sec_comment kludge is the problem, let me mention > > that different patches have been sent to glibc in order to make > > its kludge work properly(!) at least one that adds #NO_APP and > > I'm sure would work, but it seems none have been applied. > > Unfortunately, m68k and arm are the only gcc targets supported by glibc > that defaults to #NO_APP, all other targets don't have this problem. I think you forgot CRIS. BTW, I'm surprised m68k and ARM has been changed to start .s files with #NO_APP. brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 18:26 ` Hans-Peter Nilsson @ 2004-04-24 19:22 ` Andreas Schwab 2004-04-24 21:31 ` Hans-Peter Nilsson 0 siblings, 1 reply; 41+ messages in thread From: Andreas Schwab @ 2004-04-24 19:22 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: Nathan Sidwell, Ian Lance Taylor, binutils Hans-Peter Nilsson <hp@bitrange.com> writes: > I think you forgot CRIS. CRIS doesn't as of current gcc mainline, and probably never did. But VAX does. The only remaining target is NS32K, which has no glibc port. > BTW, I'm surprised m68k and ARM has > been changed to start .s files with #NO_APP. m68k always did it. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux AG, MaxfeldstraÃe 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 19:22 ` Andreas Schwab @ 2004-04-24 21:31 ` Hans-Peter Nilsson 2004-04-24 21:33 ` Andreas Schwab 0 siblings, 1 reply; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-24 21:31 UTC (permalink / raw) To: Andreas Schwab; +Cc: Nathan Sidwell, Ian Lance Taylor, binutils On Sat, 24 Apr 2004, Andreas Schwab wrote: > Hans-Peter Nilsson <hp@bitrange.com> writes: > > > I think you forgot CRIS. > > CRIS doesn't as of current gcc mainline, and probably never did. I assume you mean s/gcc/glibc/?. Oh well. Mr. Drepper never committed all the bits I sent for the CRIS port anyway and my last patches and my suggestion of becoming the maintainer were ignored. Let's hope we can communicate better next time around. Getting off-topic here... > But VAX > does. The only remaining target is NS32K, which has no glibc port. > > > BTW, I'm surprised m68k and ARM has > > been changed to start .s files with #NO_APP. > > m68k always did it. No, at least for a while it (m68k-linux) didn't. See for example the gcc-3.3 branch as of Mon Feb 23 18:58:32 GMT 2004. It does "now" (last wednesday). At a glance, it seems to have been fixed by untangling the config #include mess. I looked it up last time we had a #NO_APP issue with binutils, and it didn't at that time. ARM (arm-linux) still didn't, main trunk as of Wed Apr 21 08:31:07 GMT 2004. Not that any of this is relevant wrt. the actual incorrectness of emitting comments while in #NO_APP mode. brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 21:31 ` Hans-Peter Nilsson @ 2004-04-24 21:33 ` Andreas Schwab 2004-04-24 23:25 ` Hans-Peter Nilsson 0 siblings, 1 reply; 41+ messages in thread From: Andreas Schwab @ 2004-04-24 21:33 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: binutils Hans-Peter Nilsson <hp@bitrange.com> writes: > On Sat, 24 Apr 2004, Andreas Schwab wrote: >> Hans-Peter Nilsson <hp@bitrange.com> writes: >> >> > I think you forgot CRIS. >> >> CRIS doesn't as of current gcc mainline, and probably never did. > > I assume you mean s/gcc/glibc/?. No. gcc for cris does not (re-)define TARGET_ASM_FILE_START_APP_OFF, so the assembler never goes into NO_APP mode. >> m68k always did it. > > No, at least for a while it (m68k-linux) didn't. See for > example the gcc-3.3 branch as of Mon Feb 23 18:58:32 GMT 2004. I can't find any relevant changes in this time frame under config/m68k on the 3.3 branch. IIRC m68k-linux never redefined ASM_FILE_START and still inherits TARGET_ASM_FILE_START_APP_OFF from m68k.h. > Not that any of this is relevant wrt. the actual incorrectness > of emitting comments while in #NO_APP mode. Maybe the #APP/#NO_APP switching should be removed and input scrubbing enabled all the time, given how few targets actually disable it. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux AG, MaxfeldstraÃe 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 21:33 ` Andreas Schwab @ 2004-04-24 23:25 ` Hans-Peter Nilsson 2004-04-24 23:37 ` Andreas Schwab 2004-04-25 0:03 ` Zack Weinberg 0 siblings, 2 replies; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-24 23:25 UTC (permalink / raw) To: Andreas Schwab; +Cc: binutils On Sat, 24 Apr 2004, Andreas Schwab wrote: > Hans-Peter Nilsson <hp@bitrange.com> writes: > > On Sat, 24 Apr 2004, Andreas Schwab wrote: > >> CRIS doesn't as of current gcc mainline, and probably never did. > > I assume you mean s/gcc/glibc/?. > > No. gcc for cris does not (re-)define TARGET_ASM_FILE_START_APP_OFF, so > the assembler never goes into NO_APP mode. That conclusion is incorrect. #NO_APP for cris-* is output through default_file_start. It seems there are multiple blessed ways to get that #NO_APP out. Hmm, I thought that kind of multiplicity was something Zack W disliked but apparently not. ;-) > > No, at least for a while it (m68k-linux) didn't. See for > > example the gcc-3.3 branch as of Mon Feb 23 18:58:32 GMT 2004. > > I can't find any relevant changes in this time frame under config/m68k on > the 3.3 branch. I would have expected some #include removals. You want to look at config.gcc too. Maybe the it was all in the ASM_FILE_START revamp anyway. I compiled and checked but only guessed the reason. > IIRC m68k-linux never redefined ASM_FILE_START and still > inherits TARGET_ASM_FILE_START_APP_OFF from m68k.h. Well, compile a file and see for yourself, given the source dates I stated. Note I mentioned the 3.3 branch, where there's no TARGET_ASM_FILE_START_APP_OFF. > Maybe the #APP/#NO_APP switching should be removed and input scrubbing > enabled all the time, given how few targets actually disable it. See binutils archives from last time this came up (search for "no_app"). It saves as much as 1% off the compile time. I think *more* targets should use it, but most would need to tweak their md:s to avoid redundant spaces. brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 23:25 ` Hans-Peter Nilsson @ 2004-04-24 23:37 ` Andreas Schwab 2004-04-25 0:03 ` Zack Weinberg 1 sibling, 0 replies; 41+ messages in thread From: Andreas Schwab @ 2004-04-24 23:37 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: binutils Hans-Peter Nilsson <hp@bitrange.com> writes: > On Sat, 24 Apr 2004, Andreas Schwab wrote: >> Hans-Peter Nilsson <hp@bitrange.com> writes: >> > On Sat, 24 Apr 2004, Andreas Schwab wrote: >> >> CRIS doesn't as of current gcc mainline, and probably never did. >> > I assume you mean s/gcc/glibc/?. >> >> No. gcc for cris does not (re-)define TARGET_ASM_FILE_START_APP_OFF, so >> the assembler never goes into NO_APP mode. > > That conclusion is incorrect. #NO_APP for cris-* is output > through default_file_start. You probably mean cris_file_start. Yes, I missed that. >> > No, at least for a while it (m68k-linux) didn't. See for >> > example the gcc-3.3 branch as of Mon Feb 23 18:58:32 GMT 2004. >> >> I can't find any relevant changes in this time frame under config/m68k on >> the 3.3 branch. > > I would have expected some #include removals. You want to look > at config.gcc too. Maybe the it was all in the ASM_FILE_START > revamp anyway. I compiled and checked but only guessed the > reason. I stand corrected. ASM_FILE_START was redefined in elfos.h, already since gcc 3.1. > Note I mentioned the 3.3 branch, where there's > no TARGET_ASM_FILE_START_APP_OFF. But it's in 3.4, where it's redefined for all m68k targets. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux AG, MaxfeldstraÃe 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-24 23:25 ` Hans-Peter Nilsson 2004-04-24 23:37 ` Andreas Schwab @ 2004-04-25 0:03 ` Zack Weinberg 2004-04-25 0:22 ` Hans-Peter Nilsson 2004-04-25 23:35 ` Ian Lance Taylor 1 sibling, 2 replies; 41+ messages in thread From: Zack Weinberg @ 2004-04-25 0:03 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: Andreas Schwab, binutils Hans-Peter Nilsson <hp@bitrange.com> writes: > That conclusion is incorrect. #NO_APP for cris-* is output > through default_file_start. It seems there are multiple blessed > ways to get that #NO_APP out. Hmm, I thought that kind of > multiplicity was something Zack W disliked but apparently not. ;-) I *tried*. Y'all keep moving the goalposts. ;-) >> Maybe the #APP/#NO_APP switching should be removed and input scrubbing >> enabled all the time, given how few targets actually disable it. > > See binutils archives from last time this came up (search for > "no_app"). It saves as much as 1% off the compile time. I > think *more* targets should use it, but most would need to tweak > their md:s to avoid redundant spaces. Frankly, (as someone who has to stare at GCC's assembly output all the damn time), I do not think 1% speedup is worth the *severe* degradation in readability that this would impose. I would much rather put effort into making GAS's parser be faster in input-scrubbing mode. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-25 0:03 ` Zack Weinberg @ 2004-04-25 0:22 ` Hans-Peter Nilsson 2004-04-26 0:28 ` Zack Weinberg 2004-04-25 23:35 ` Ian Lance Taylor 1 sibling, 1 reply; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-25 0:22 UTC (permalink / raw) To: Zack Weinberg; +Cc: Andreas Schwab, binutils On Sat, 24 Apr 2004, Zack Weinberg wrote: > Hans-Peter Nilsson <hp@bitrange.com> writes: > > See binutils archives from last time this came up (search for > > "no_app"). It saves as much as 1% off the compile time. I > > think *more* targets should use it, but most would need to tweak > > their md:s to avoid redundant spaces. > > Frankly, (as someone who has to stare at GCC's assembly output all the > damn time), I do not think 1% speedup is worth the *severe* > degradation in readability that this would impose. I would much > rather put effort into making GAS's parser be faster in > input-scrubbing mode. I think you overrate the amount of change needed. As another person who has to stare at GCC's assembly output all the (...) time, my take is that it would (if anything) improve readability ;-) but most problably it's neutral maintainerwise. All it takes is to change some spacing like "move.d r0, r1" to move.d r0,r1" and remove spurious (often just extraneous) spacing in target-defined pseudoopcodes and rarely used constructs like the trampoline template. Consider checking cris.md and pointing out what #NO_APP-related spacing makes it "severely" unreadable! Not that speeding up GAS parsing would be a bad thing, of course. brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-25 0:22 ` Hans-Peter Nilsson @ 2004-04-26 0:28 ` Zack Weinberg 2004-04-26 0:58 ` Hans-Peter Nilsson 0 siblings, 1 reply; 41+ messages in thread From: Zack Weinberg @ 2004-04-26 0:28 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: Andreas Schwab, binutils Hans-Peter Nilsson <hp@bitrange.com> writes: > All it takes is to change some spacing like "move.d r0, r1" to > move.d r0,r1" and remove spurious (often just extraneous) > spacing in target-defined pseudoopcodes and rarely used > constructs like the trampoline template. Well, yeah, see, i386.md was changed from the #NO_APP-compatible "mov %0,%1" to the current "mov\t%0, %1" in the egcs 1.1 timeframe, if I recall right, and so we've had a chance to try the same assembly language both ways, and I vastly prefer it with the tab and the space. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 0:28 ` Zack Weinberg @ 2004-04-26 0:58 ` Hans-Peter Nilsson 2004-04-26 2:14 ` Hans-Peter Nilsson 0 siblings, 1 reply; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-26 0:58 UTC (permalink / raw) To: Zack Weinberg; +Cc: Andreas Schwab, binutils On Sun, 25 Apr 2004, Zack Weinberg wrote: > Hans-Peter Nilsson <hp@bitrange.com> writes: > > > All it takes is to change some spacing like "move.d r0, r1" to > > move.d r0,r1" and remove spurious (often just extraneous) > > spacing in target-defined pseudoopcodes and rarely used > > constructs like the trampoline template. > > Well, yeah, see, i386.md was changed from the #NO_APP-compatible > "mov %0,%1" to the current "mov\t%0, %1" in the egcs 1.1 timeframe, if Right, IIRC I even mentioned i386.md last time. :-) The i386 backend rewrite (going from CC0) had this #NO_APP-incompatibility as an extra at no additional charge. > I recall right, and so we've had a chance to try the same assembly > language both ways, and I vastly prefer it with the tab and the space. FWIW, I think you can have the TAB, but not the extra space in the parameters. The lack of this extra space (and/or the TAB) is a "*severe* degradation in readability"? Hmm. I believe we've reduced this issue to a matter of taste. (Again, methinks -- same conclusion as last time.) brgds, H-P PS "Prescrubbed" is a misnomer, certainly about the #NO_APP strict format. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 0:58 ` Hans-Peter Nilsson @ 2004-04-26 2:14 ` Hans-Peter Nilsson 0 siblings, 0 replies; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-26 2:14 UTC (permalink / raw) To: Zack Weinberg; +Cc: Andreas Schwab, binutils On Sun, 25 Apr 2004, Hans-Peter Nilsson wrote: > > I recall right, and so we've had a chance to try the same assembly > > language both ways, and I vastly prefer it with the tab and the space. > > FWIW, I think you can have the TAB, but not the extra space in > the parameters. Nope, sorry for the misinformation. For the old discussion, see <URL:http://gcc.gnu.org/ml/gcc-patches/2003-06/msg02565.html>. (And it's 2% not 1%.) brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-25 0:03 ` Zack Weinberg 2004-04-25 0:22 ` Hans-Peter Nilsson @ 2004-04-25 23:35 ` Ian Lance Taylor 2004-04-26 0:51 ` Zack Weinberg 1 sibling, 1 reply; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-25 23:35 UTC (permalink / raw) To: Zack Weinberg; +Cc: Hans-Peter Nilsson, Andreas Schwab, binutils Zack Weinberg <zack@codesourcery.com> writes: > Frankly, (as someone who has to stare at GCC's assembly output all the > damn time), I do not think 1% speedup is worth the *severe* > degradation in readability that this would impose. I would much > rather put effort into making GAS's parser be faster in > input-scrubbing mode. I've certainly looked at plenty of assembler code, and I personally don't think there is any degradation in readability whatsoever. The only change of any significance is that comments are not permitted. It's not difficult to generate comments only when some option is used--such as -fverbose-asm. Can you give some examples of how pre-scrubbed code is harder to read? I put a lot of time into speeding up input scrubbing a few years back, and got some good results, which are now part of gas. But no matter what we do it's going to be faster to process code if we don't have to skip spaces and comments. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-25 23:35 ` Ian Lance Taylor @ 2004-04-26 0:51 ` Zack Weinberg 2004-04-26 2:46 ` Ian Lance Taylor 0 siblings, 1 reply; 41+ messages in thread From: Zack Weinberg @ 2004-04-26 0:51 UTC (permalink / raw) To: hp; +Cc: schwab, binutils Ian Lance Taylor <ian@wasabisystems.com> writes: > Can you give some examples of how pre-scrubbed code is harder to read? For me, it really is all about the whitespace. Here's a fragment of x86 assembly as currently generated - main: pushl %ebp movl $1, %eax movl %esp, %ebp pushl %ebx subl $36, %esp andl $-16, %esp movl %eax, 4(%esp) movl $13, (%esp) call signal movl $17, (%esp) movl $1, %eax movl %eax, 4(%esp) call signal and as it would be if prescrubbed: main: pushl %ebp movl $1,%eax movl %esp,%ebp pushl %ebx subl $36,%esp andl $-16,%esp movl %eax,4(%esp) movl $13,(%esp) call signal movl $17,(%esp) movl $1,%eax movl %eax,4(%esp) call signal The denser packing, and the lack of a nicely lined up arguments field, mean substantially more mental effort just to see what it's doing. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 0:51 ` Zack Weinberg @ 2004-04-26 2:46 ` Ian Lance Taylor 2004-04-26 3:13 ` Zack Weinberg 0 siblings, 1 reply; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-26 2:46 UTC (permalink / raw) To: Zack Weinberg; +Cc: hp, schwab, binutils Zack Weinberg <zack@codesourcery.com> writes: > The denser packing, and the lack of a nicely lined up arguments field, > mean substantially more mental effort just to see what it's doing. I'd say you're making a mountain out of a molehill. You're condemning the entire world to have slower compiles because you don't want to work a little bit harder to read assembler code, nor to write a sed script to do beautification, nor to use the -fverbose-asm option. If there is really a 1% speedup with #NO_APP, then it might even save you time overall due to faster compiles, even at the cost of spending more time in the relatively rare cases that you have to look at i386 assembler output. I'm sure it would save time for everybody else. For that matter, this patch to gas would accept your example input in #NO_APP mode. Ian Index: tc-i386.c =================================================================== RCS file: /cvs/src/src/gas/config/tc-i386.c,v retrieving revision 1.150 diff -u -r1.150 tc-i386.c --- tc-i386.c 12 Mar 2004 10:14:29 -0000 1.150 +++ tc-i386.c 26 Apr 2004 02:05:49 -0000 @@ -237,6 +237,7 @@ #define is_operand_char(x) (operand_chars[(unsigned char) x]) #define is_register_char(x) (register_chars[(unsigned char) x]) #define is_space_char(x) ((x) == ' ') +#define is_space_or_tab_char(x) ((x) == ' ' || (x) == '\t') #define is_identifier_char(x) (identifier_chars[(unsigned char) x]) #define is_digit_char(x) (digit_chars[(unsigned char) x]) @@ -1506,7 +1507,7 @@ } l++; } - if (!is_space_char (*l) + if (!is_space_or_tab_char (*l) && *l != END_OF_INSN && *l != PREFIX_SEPARATOR && *l != ',') @@ -1528,7 +1529,7 @@ current_templates = hash_find (op_hash, mnemonic); if (*l != END_OF_INSN - && (!is_space_char (*l) || l[1] != END_OF_INSN) + && (!is_space_or_tab_char (*l) || l[1] != END_OF_INSN) && current_templates && (current_templates->start->opcode_modifier & IsPrefix)) { @@ -1673,7 +1674,7 @@ while (*l != END_OF_INSN) { /* Skip optional white space before operand. */ - if (is_space_char (*l)) + if (is_space_or_tab_char (*l)) ++l; if (!is_operand_char (*l) && *l != END_OF_INSN) { ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 2:46 ` Ian Lance Taylor @ 2004-04-26 3:13 ` Zack Weinberg 2004-04-26 14:16 ` Ian Lance Taylor 0 siblings, 1 reply; 41+ messages in thread From: Zack Weinberg @ 2004-04-26 3:13 UTC (permalink / raw) To: hp; +Cc: schwab, binutils Ian Lance Taylor <ian@wasabisystems.com> writes: > Zack Weinberg <zack@codesourcery.com> writes: > >> The denser packing, and the lack of a nicely lined up arguments field, >> mean substantially more mental effort just to see what it's doing. > > I'd say you're making a mountain out of a molehill. You're condemning > the entire world to have slower compiles because you don't want to > work a little bit harder to read assembler code, nor to write a sed > script to do beautification, nor to use the -fverbose-asm option. The conversation so far was mainly about the appearance of the generated assembly, which I agree is minor, but I have another reason for not liking #NO_APP, which is that it adds a nontrivial amount of complexity to GCC's assembly output logic. I would like if we could make it all just go away. > For that matter, this patch to gas would accept your example input in > #NO_APP mode. This is the sort of thing I was thinking about when I said 'let's spend time making GAS go faster instead' -- although, if I were in charge of doing that, I'd try to collapse as much of the parsing logic together as possible, rather than tweaking all the tc-* parsers. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 3:13 ` Zack Weinberg @ 2004-04-26 14:16 ` Ian Lance Taylor 2004-04-26 14:22 ` Andreas Schwab ` (2 more replies) 0 siblings, 3 replies; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-26 14:16 UTC (permalink / raw) To: Zack Weinberg; +Cc: hp, schwab, binutils Zack Weinberg <zack@codesourcery.com> writes: > The conversation so far was mainly about the appearance of the > generated assembly, which I agree is minor, but I have another > reason for not liking #NO_APP, which is that it adds a nontrivial > amount of complexity to GCC's assembly output logic. I would like > if we could make it all just go away. It does add complexity where we want -fverbose-asm support. Other than that, as far as I can see, it does not. Is that the nontrivial complexity that you are talking about? > > For that matter, this patch to gas would accept your example input in > > #NO_APP mode. > > This is the sort of thing I was thinking about when I said 'let's > spend time making GAS go faster instead' -- although, if I were in > charge of doing that, I'd try to collapse as much of the parsing logic > together as possible, rather than tweaking all the tc-* parsers. Well, the assembler parsing logic is already completely collapsed together. It is all done in gas/app.c, using a state machine that operates on buffers. (It turns out that there are a number of machine dependencies for assembler parsing, but they are still all handled in one place.) The output of do_scrub_chars() in gas/app.c is what the machine dependent parsers work with. The idea behind #NO_APP is that the assembler input is precisely that which is generated by do_scrub_chars(). Thus, in #NO_APP mode, do_scrub_chars() is not called at all. do_scrub_chars() will reliably convert sequences of whitespace to a single space, which is why the i386 assembler can reliably use is_space_char() to skip whitespace, and that is why #NO_APP code is expected to use a single space character. My patch to config/tc-i386.c was to tweak what the i386 assembler will accept so that it would take the tab characters you wanted. I agree that this approach is not particularly useful. The general #NO_APP approach makes a great deal of sense to me. 99% of the time the assembler code generated by the compiler is read only by the assembler. Given that, it is already kind of silly that the compiler has to go to a lot of effort to generate a text file. The assembler has to spend time doing parsing and hash table lookups to recreate information which the compiler already has. Given that we are going to stick with a text file to communicate between the compiler and the assembler, it makes sense to me to eliminate parsing in the assembler as much as possible. We have text generated by one program and read by another program; parsing is useless overhead. That is the idea behind #NO_APP. Naturally an even more efficient process would be to link the compiler and the assembler together into one process, or to use a binary communication format which does not require parsing. (The latter is what the MIPS compiler from MIPS used to do, and perhaps still does.) However, both of those would require a great deal more engineering effort than #NO_APP. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 14:16 ` Ian Lance Taylor @ 2004-04-26 14:22 ` Andreas Schwab 2004-04-26 14:34 ` Richard Earnshaw 2004-04-26 19:42 ` Kai Henningsen 2004-04-27 1:32 ` Zack Weinberg 2 siblings, 1 reply; 41+ messages in thread From: Andreas Schwab @ 2004-04-26 14:22 UTC (permalink / raw) To: hp; +Cc: binutils, zack Ian Lance Taylor <ian@wasabisystems.com> writes: > Given that we are going to stick with a text file to communicate > between the compiler and the assembler, it makes sense to me to > eliminate parsing in the assembler as much as possible. We have text > generated by one program and read by another program; parsing is > useless overhead. That is the idea behind #NO_APP. Currently (gcc 3.4.0) there are 5 targets that actually make use of #NO_APP: arm, cris, m68k, ns32k and vax (and m68k-linux didn't in gcc 3.0.x upto 3.3.x due to a side effect of including "elfos.h", which redefines ASM_FILE_START). Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux AG, MaxfeldstraÃe 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 14:22 ` Andreas Schwab @ 2004-04-26 14:34 ` Richard Earnshaw 2004-04-26 15:29 ` Ian Lance Taylor 0 siblings, 1 reply; 41+ messages in thread From: Richard Earnshaw @ 2004-04-26 14:34 UTC (permalink / raw) To: Andreas Schwab; +Cc: hp, binutils, zack On Mon, 2004-04-26 at 15:19, Andreas Schwab wrote: > Ian Lance Taylor <ian@wasabisystems.com> writes: > > > Given that we are going to stick with a text file to communicate > > between the compiler and the assembler, it makes sense to me to > > eliminate parsing in the assembler as much as possible. We have text > > generated by one program and read by another program; parsing is > > useless overhead. That is the idea behind #NO_APP. > > Currently (gcc 3.4.0) there are 5 targets that actually make use of > #NO_APP: arm, cris, m68k, ns32k and vax (and m68k-linux didn't in gcc > 3.0.x upto 3.3.x due to a side effect of including "elfos.h", which > redefines ASM_FILE_START). That's probably misleading. The gcc might emit a directive for that, but I'm almost certain the ARM assembler for one takes no notice of it. R. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 14:34 ` Richard Earnshaw @ 2004-04-26 15:29 ` Ian Lance Taylor 2004-04-26 19:26 ` Hans-Peter Nilsson 0 siblings, 1 reply; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-26 15:29 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Andreas Schwab, hp, binutils, zack Richard Earnshaw <Richard.Earnshaw@arm.com> writes: > > Currently (gcc 3.4.0) there are 5 targets that actually make use of > > #NO_APP: arm, cris, m68k, ns32k and vax (and m68k-linux didn't in gcc > > 3.0.x upto 3.3.x due to a side effect of including "elfos.h", which > > redefines ASM_FILE_START). > > That's probably misleading. The gcc might emit a directive for that, > but I'm almost certain the ARM assembler for one takes no notice of it. The code to handle #NO_APP in gas is independent of the CPU backend. The initial #NO_APP comment is recognized in input_file_open(); it must be the first characters in the file. It then turns off preprocessing (i.e., calls to do_scrub_chars) except for code which follows #APP up to #NO_APP. That said, I don't see #NO_APP in assembler code generated by an xscale-elf compiler. In fact, I see that TARGET_ASM_FILE_START_APP_OFF is defined to be true, which should generate #NO_APP, but that ASM_APP_OFF is defined to be "", so the effect is nullified. That is, the ARM compiler does not generally use #NO_APP. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 15:29 ` Ian Lance Taylor @ 2004-04-26 19:26 ` Hans-Peter Nilsson 0 siblings, 0 replies; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-26 19:26 UTC (permalink / raw) To: binutils On Mon, 26 Apr 2004, Ian Lance Taylor wrote: > Richard Earnshaw <Richard.Earnshaw@arm.com> writes: > Andreas Schwab wrote: > > > Currently (gcc 3.4.0) there are 5 targets that actually make use of > > > #NO_APP: arm, cris, m68k, ns32k and vax (and m68k-linux didn't in gcc > > > 3.0.x upto 3.3.x due to a side effect of including "elfos.h", which > > > redefines ASM_FILE_START). > > > > That's probably misleading. The gcc might emit a directive for that, > > but I'm almost certain the ARM assembler for one takes no notice of it. > > The code to handle #NO_APP in gas is independent of the CPU backend. > The initial #NO_APP comment is recognized in input_file_open(); it > must be the first characters in the file. It then turns off > preprocessing (i.e., calls to do_scrub_chars) except for code which > follows #APP up to #NO_APP. > > That said, I don't see #NO_APP in assembler code generated by an > xscale-elf compiler. (I already mentioned, but some apparently missed it or --hopefully not-- disbelieved it, so let me repeat anyway, that) I checked arm-linux main trunk, confirming that *no* #NO_APP is generated at the top of the generated assembly files. I'm reluctant to accept the list above (except for CRIS and m68k from previous checking) without checking myself. ;-) brgds, H-P ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 14:16 ` Ian Lance Taylor 2004-04-26 14:22 ` Andreas Schwab @ 2004-04-26 19:42 ` Kai Henningsen 2004-04-26 19:45 ` Ian Lance Taylor 2004-04-27 1:32 ` Zack Weinberg 2 siblings, 1 reply; 41+ messages in thread From: Kai Henningsen @ 2004-04-26 19:42 UTC (permalink / raw) To: binutils ian@wasabisystems.com (Ian Lance Taylor) wrote on 26.04.04 in <m3wu42am0e.fsf@gossamer.airs.com>: > Well, the assembler parsing logic is already completely collapsed > together. It is all done in gas/app.c, using a state machine that > operates on buffers. (It turns out that there are a number of machine > dependencies for assembler parsing, but they are still all handled in > one place.) The output of do_scrub_chars() in gas/app.c is what the > machine dependent parsers work with. Has anyone tried including the effects of do_scrub_chars() into that state machine? It certainly sounds as if that ought to be entirely possible _and_ fast. Whereas the current two-pass logic sounds rather slow. MfG Kai ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 19:42 ` Kai Henningsen @ 2004-04-26 19:45 ` Ian Lance Taylor 2004-04-26 20:04 ` Ian Lance Taylor 0 siblings, 1 reply; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-26 19:45 UTC (permalink / raw) To: Kai Henningsen; +Cc: binutils kaih@khms.westfalen.de (Kai Henningsen) writes: > ian@wasabisystems.com (Ian Lance Taylor) wrote on 26.04.04 in <m3wu42am0e.fsf@gossamer.airs.com>: > > > Well, the assembler parsing logic is already completely collapsed > > together. It is all done in gas/app.c, using a state machine that > > operates on buffers. (It turns out that there are a number of machine > > dependencies for assembler parsing, but they are still all handled in > > one place.) The output of do_scrub_chars() in gas/app.c is what the > > machine dependent parsers work with. > > Has anyone tried including the effects of do_scrub_chars() into that state > machine? It certainly sounds as if that ought to be entirely possible > _and_ fast. Whereas the current two-pass logic sounds rather slow. I guess I was unclear. do_scrub_chars() is the state machine. The parsing is indeed two pass. All the input (other than #NO_APP code) is first run through the do_scrub_chars() state machine. The general parsing code then looks at the parsed input. Basically the general parsing code in read_a_source_file() requests a buffer from input_scrub_next_buffer(). That function calls input_file_give_next_buffer() to read data. That function calls either fread() or do_scrub_chars(), depending upon whether #NO_APP is in effect. If do_scrub_chars() is called, it calls fread(), scrubs it, and returns the resulting buffer. input_scrub_next_buffer() only returns complete lines. The general parsing code parses all the information in the buffer, then requests another buffer. And so forth. This is all made more complex by handling macros and include files. The general parsing code then checks for a pseudo-op (generally, a leading '.' indicates a pseudo-op) and handles pseudo-ops in a more or less machine independent fashion. Anything else is passed to the machine dependent parser. The code could certainly be cleaned up, as is true of most of gas. But I don't see anything wrong with the underlying logic. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 19:45 ` Ian Lance Taylor @ 2004-04-26 20:04 ` Ian Lance Taylor 0 siblings, 0 replies; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-26 20:04 UTC (permalink / raw) To: binutils Ian Lance Taylor <ian@wasabisystems.com> writes: > The code could certainly be cleaned up, as is true of most of gas. > But I don't see anything wrong with the underlying logic. Actually, there is one obvious simplification we could apply: the assembler could always read the entire input file into memory, and process it that way, rather than reading it buffer by buffer. That would simplify and speed up the state machine logic. However, that would in some cases be a significant change to the memory requirements of gas. For example, the i386 insn-attrtab.s file is 2.7M on my system, which is a lot on some systems. On the other hand, the assembler builds other data structures which are proportional to the size of the input file. So memory requirements will not increase by an order of magnitude, or anything like that. So it might be acceptable. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-26 14:16 ` Ian Lance Taylor 2004-04-26 14:22 ` Andreas Schwab 2004-04-26 19:42 ` Kai Henningsen @ 2004-04-27 1:32 ` Zack Weinberg 2004-04-27 2:02 ` Hans-Peter Nilsson ` (2 more replies) 2 siblings, 3 replies; 41+ messages in thread From: Zack Weinberg @ 2004-04-27 1:32 UTC (permalink / raw) To: hp; +Cc: schwab, binutils Ian Lance Taylor <ian@wasabisystems.com> writes: > Zack Weinberg <zack@codesourcery.com> writes: > >> The conversation so far was mainly about the appearance of the >> generated assembly, which I agree is minor, but I have another >> reason for not liking #NO_APP, which is that it adds a nontrivial >> amount of complexity to GCC's assembly output logic. I would like >> if we could make it all just go away. > > It does add complexity where we want -fverbose-asm support. Other > than that, as far as I can see, it does not. Is that the nontrivial > complexity that you are talking about? I'm talking about the 192 places in the GCC source code where "APP" or "NO_APP" or other names related to emitting these strings in assembly appear. If it were one, I'd not be complaining. Two or three even, ok. 192, not cool. (number got by running "find . -type f | grep -v CVS | grep -v ChangeLog | xargs egrep '(\<APP\>|\<NO_APP\>|APP_ON|APP_OFF|app_on|app_off)' | wc -l" in a CVS checkout - yes, documentation deliberately included). >> This is the sort of thing I was thinking about when I said 'let's >> spend time making GAS go faster instead' -- although, if I were in >> charge of doing that, I'd try to collapse as much of the parsing logic >> together as possible, rather than tweaking all the tc-* parsers. > > Well, the assembler parsing logic is already completely collapsed > together. It is all done in gas/app.c, using a state machine that > operates on buffers. (It turns out that there are a number of machine > dependencies for assembler parsing, but they are still all handled in > one place.) The output of do_scrub_chars() in gas/app.c is what the > machine dependent parsers work with. Uh, when I said "collapse the parsing logic together" I meant "think of a way to get rid of the machine-dependent parsers to the maximum extent possible." While thinking very carefully about how to make the parser go blazingly fast *without* special tricks. I'm quite confident this is possible. Assembly syntax - with or without extra added whitespace - is not complicated, nor does it vary that much across architectures supported by GAS. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 1:32 ` Zack Weinberg @ 2004-04-27 2:02 ` Hans-Peter Nilsson 2004-04-27 2:38 ` Zack Weinberg 2004-04-27 2:35 ` Alan Modra 2004-04-27 2:47 ` Ian Lance Taylor 2 siblings, 1 reply; 41+ messages in thread From: Hans-Peter Nilsson @ 2004-04-27 2:02 UTC (permalink / raw) To: Zack Weinberg; +Cc: schwab, binutils On Mon, 26 Apr 2004, Zack Weinberg wrote: > I'm talking about the 192 places in the GCC source code where "APP" or > "NO_APP" or other names related to emitting these strings in assembly > appear. If it were one, I'd not be complaining. Two or three even, > ok. 192, not cool. > > (number got by running "find . -type f | grep -v CVS | grep -v ChangeLog | > xargs egrep '(\<APP\>|\<NO_APP\>|APP_ON|APP_OFF|app_on|app_off)' | wc -l" > in a CVS checkout - yes, documentation deliberately included). Seems you're counting occurrences in individual target files as well. Besides, IIRC there's redundancy in the machinery, some of which you added yourself. If you remove it all from core GCC, we can still have the #NO_APP emitted in the target's file_start_whateveritsname function. I really don't see why there should *necessarily* be a "nontrivial" overhead. (Away from my checkout areas at the moment so I can't produce counterclaims. ;-) Point is, this functionality, emitting a string at the head of the assembly file, and avoiding it if flag_verbose_asm, really doesn't have to be complex... brgds, H-P PS. Replies to Ian's messages be better sent to Ian than me. Maybe the mailer is playing tricks on me but that'd be a first. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 2:02 ` Hans-Peter Nilsson @ 2004-04-27 2:38 ` Zack Weinberg 0 siblings, 0 replies; 41+ messages in thread From: Zack Weinberg @ 2004-04-27 2:38 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: schwab, binutils Hans-Peter Nilsson <hp@bitrange.com> writes: > On Mon, 26 Apr 2004, Zack Weinberg wrote: >> I'm talking about the 192 places in the GCC source code where "APP" or >> "NO_APP" or other names related to emitting these strings in assembly >> appear. If it were one, I'd not be complaining. Two or three even, >> ok. 192, not cool. >> >> (number got by running "find . -type f | grep -v CVS | grep -v ChangeLog | >> xargs egrep '(\<APP\>|\<NO_APP\>|APP_ON|APP_OFF|app_on|app_off)' | wc -l" >> in a CVS checkout - yes, documentation deliberately included). > > Seems you're counting occurrences in individual target files as > well. Quite deliberately. I said I'd not be complaining if this crap appeared in one place, and I meant one place. In the entire source tree. Which is hyperbole; but I do think the impact on the sources could be narrowed down quite a bit, along the following lines: - completely decouple it from TARGET_ASM_FILE_START. GAS wants to see it as the very first thing in the file, and it's supposed to be a comment so no one else should care if it e.g. comes ahead of a .file directive. So emit it from init_asm_output before targetm.asm_out.file_start. Actually, fold all of default_file_start into init_asm_output. - eliminate ASM_APP_ON/OFF entirely; the words "NO_APP" and "APP" should be invariant, the comment character should be ASM_COMMENT_START (which ought to get target-hook-ized with a sensible default, "#" looks good). - think of a saner way to do what alpha.h is currently doing with ASM_APP_ON/OFF. - overhaul final_scan_insn so it's not such a horrible mess. But I'd rather spend time thinking about how to make GAS faster so it can all go away. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 1:32 ` Zack Weinberg 2004-04-27 2:02 ` Hans-Peter Nilsson @ 2004-04-27 2:35 ` Alan Modra 2004-04-27 3:13 ` Zack Weinberg 2004-04-27 2:47 ` Ian Lance Taylor 2 siblings, 1 reply; 41+ messages in thread From: Alan Modra @ 2004-04-27 2:35 UTC (permalink / raw) To: Zack Weinberg; +Cc: hp, schwab, binutils On Mon, Apr 26, 2004 at 05:51:44PM -0700, Zack Weinberg wrote: > Uh, when I said "collapse the parsing logic together" I meant "think > of a way to get rid of the machine-dependent parsers to the maximum > extent possible." While thinking very carefully about how to make the > parser go blazingly fast *without* special tricks. I'm quite > confident this is possible. Assembly syntax - with or without extra > added whitespace - is not complicated, nor does it vary that much > across architectures supported by GAS. One thing that complicates do_scrub_chars in gas, for very little gain, is trying to remove all unneeded whitespace. For example, foo: mov %eax, $2 + 3 + 4 is turned into foo: mov %eax,$2+3+4 The aim being to render whitespace removal unnecessary in target dependent code. That's a good idea until you encounter constructs like: foo: addr16 mov %eax,0 where the "opcode" part of the instruction itself has whitespace. Without various hacks, do_scrub_chars would turn the above into foo: addr16 mov%eax,0 Other architectures have similar assembly constructs. So I think you underestimate the complexity in a general parser design. Just separating an assembly line into label, opcode, operands isn't so easy! -- Alan Modra IBM OzLabs - Linux Technology Centre ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 2:35 ` Alan Modra @ 2004-04-27 3:13 ` Zack Weinberg 2004-04-27 4:33 ` Ian Lance Taylor 0 siblings, 1 reply; 41+ messages in thread From: Zack Weinberg @ 2004-04-27 3:13 UTC (permalink / raw) To: hp; +Cc: schwab, binutils Alan Modra <amodra@bigpond.net.au> writes: ... > foo: addr16 mov %eax,0 > > where the "opcode" part of the instruction itself has whitespace. > Without various hacks, do_scrub_chars would turn the above into > > foo: addr16 mov%eax,0 > > Other architectures have similar assembly constructs. So I think you > underestimate the complexity in a general parser design. Just > separating an assembly line into label, opcode, operands isn't so easy! Eh, that doesn't look so bad to me, if the set of opcodes and the set of prefixes is made available to the generic parser. The basic grammar is something like line: label | label? WS (prefix WS)* opcode WS operands | WS? directive operands operands: operand | operands WS? ',' WS? operand where the sets of prefixes, opcodes, and directives are known well in advance - this might be a good application for perfect hashing. Operand parsing might have to stay machine-specific, but I hope not, the commonalities are huge. And then there are additional little bits of goo that some architectures let in, like IA64 and its punctuation - I would handle this by treating those as directives, which is why the above grammar doesn't separate the '.' from the directive. Macro handling (I mean .macro, not the built-in pseudo-opcodes that some architectures have) is awkward because you can override opcodes with macros. Not sure what to do about that. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 3:13 ` Zack Weinberg @ 2004-04-27 4:33 ` Ian Lance Taylor 2004-04-27 5:19 ` Zack Weinberg 2004-04-27 11:43 ` Richard Earnshaw 0 siblings, 2 replies; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-27 4:33 UTC (permalink / raw) To: Zack Weinberg; +Cc: hp, schwab, binutils Zack Weinberg <zack@codesourcery.com> writes: I already mentioned some difficulties, here are just a few more. > Eh, that doesn't look so bad to me, if the set of opcodes and the set > of prefixes is made available to the generic parser. The basic > grammar is something like > > line: label > | label? WS (prefix WS)* opcode WS operands > | WS? directive operands > > operands: operand > | operands WS? ',' WS? operand You left out lines like "foo = 4". You left out TI label pseudo-op syntax: "foo: .struct ..." in which the label must be handled differently. You left out MIPS .loc which does not use commas between operands. You left out operand syntax like ', (single quote followed by comma) which is supported by some targets but not others. You left out the truly dreadful ARM .symver, in which '@' starts a comment at any time--except when it appears in the operand of .symver. And, repeating myself, MRI mode. > where the sets of prefixes, opcodes, and directives are known well in > advance - this might be a good application for perfect hashing. The set of opcodes varies at run time, sometimes significantly as for PPC or MIPS 32-bit mode vs. 64-bit mode. However, it would probably be acceptable to build a perfect hash and handle the results after lookup. > I would handle this by treating those as directives, which is why the > above grammar doesn't separate the '.' from the directive. A good thing, too, since on m88k, and MRI mode, and some m68k variants, the '.' is optional. > Macro handling (I mean .macro, not the built-in pseudo-opcodes that > some architectures have) is awkward because you can override opcodes > with macros. Not sure what to do about that. Actually, that is relatively easy. If any macros have been defined, you have to look up the directive name in the macro hash table before you look it up in the other hash table. Macros don't normally appear in compiler output, so speed is not as important here. Again, can these issues be solved? Yes. Is this obviously going to make everything better? No. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 4:33 ` Ian Lance Taylor @ 2004-04-27 5:19 ` Zack Weinberg 2004-04-27 6:52 ` Ian Lance Taylor 2004-04-27 11:43 ` Richard Earnshaw 1 sibling, 1 reply; 41+ messages in thread From: Zack Weinberg @ 2004-04-27 5:19 UTC (permalink / raw) To: binutils Ian Lance Taylor <ian@wasabisystems.com> writes: > Zack Weinberg <zack@codesourcery.com> writes: > > I already mentioned some difficulties, here are just a few more. [...] I'm going to take this as "put up or shut up", and indeed I would not have blathered on about this for so long if it weren't that I expect to get a chance to do some performance work on GAS fairly soon (by which I mean "sometime this year"). I'm going to drop the thread now, and come back when I have actual code to show people. zw ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 5:19 ` Zack Weinberg @ 2004-04-27 6:52 ` Ian Lance Taylor 0 siblings, 0 replies; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-27 6:52 UTC (permalink / raw) To: Zack Weinberg; +Cc: binutils Zack Weinberg <zack@codesourcery.com> writes: > I'm going to take this as "put up or shut up", and indeed I would not > have blathered on about this for so long if it weren't that I expect > to get a chance to do some performance work on GAS fairly soon (by > which I mean "sometime this year"). I'm going to drop the thread now, > and come back when I have actual code to show people. I'm glad that you will have some time to work on this. For what it's worth, I think the thing which most needs fixing in gas is the relocation handling, which is completely and horribly broken. However, that is not a performance issue. For performance issues, I'll note that assembler relaxation currently walks the entire frag chain at least three times. I'm sure a more clever algorithm is possible. I wouldn't be surprised if some more code is quietly walking long lists somewhere. It might be beneficial to change the hash table to use libiberty/hashtab.c; I don't know. The last time I did serious performance hacking on gas, I did the following. I uncovered this mostly by using profiling and examing memory usage. Ian http://sources.redhat.com/ml/binutils/1999-q2/msg00108.html 1999-06-03 Ian Lance Taylor <ian@zembu.com> Add support for storing local symbols in a small structure to save memory when assembling large files. * as.h: Don't include struc-symbol.h. (symbolS): Add typedef. * symbols.c: Include struc-symbol.h. (local_hash): New static variable. (save_symbol_name): New static function, from symbol_create. (symbol_create): Call save_symbol_name. (local_symbol_count): New static variable. (local_symbol_conversion_count): Likewise. (LOCAL_SYMBOL_CHECK): Define. (local_symbol_make): New static function. (local_symbol_convert): New static function. (colon): Handle local symbols. Create local symbol for local label name. (symbol_table_insert): Handle local symbols. (symbol_find_or_make): Create local symbol for local label name. (symbol_find_base): Check for local symbol. (symbol_append, symbol_insert): Check for local symbols. (symbol_clear_list_pointers, symbol_remove): Likewise. (verify_symbol_chain): Likewise. (copy_symbol_attributes): Likewise. (resolve_symbol_value): Handle local symbols. (resolve_local_symbol): New static function. (resolve_local_symbol_values): New function. (S_GET_VALUE, S_SET_VALUE): Handle local symbols. (S_IS_FUNCTION, S_IS_EXTERNAL, S_IS_WEAK, S_IS_COMMON): Likewise. (S_IS_DEFINED, S_IS_DEBUG, S_IS_LOCAL, S_GET_NAME): Likewise. (S_GET_SEGMENT, S_SET_SEGMENT, S_SET_EXTERNAL): Likewise. (S_CLEAR_EXTERNAL, S_SET_WEAK, S_SET_NAME): Likewise. (symbol_previous, symbol_next): New functions. (symbol_get_value_expression): Likewise. (symbol_set_value_expression): Likewise. (symbol_set_frag, symbol_get_frag): Likewise. (symbol_mark_used, symbol_clear_used, symbol_used_p): Likewise. (symbol_mark_used_in_reloc): Likewise. (symbol_clear_used_in_reloc, symbol_used_in_reloc_p): Likewise. (symbol_mark_mri_common, symbol_clear_mri_common): Likewise. (symbol_mri_common_p): Likewise. (symbol_mark_written, symbol_clear_written): Likewise. (symbol_written_p): Likewise. (symbol_mark_resolved, symbol_resolved_p): Likewise. (symbol_section_p, symbol_equated_p): Likewise. (symbol_constant_p): Likewise. (symbol_get_bfdsym, symbol_set_bfdsym): Likewise. (symbol_get_obj, symbol_set_obj): Likewise. (symbol_get_tc, symbol_set_tc): Likewise. (symbol_begin): Initialize local_hash. (print_symbol_value_1): Handle local symbols. (symbol_print_statistics): Print local symbol statistics. * symbols.h: Include "struc-symbol.h" if not BFD_ASSEMBLER. Declare new symbols.c functions. Move many declarations here from struc-symbol.h. (SYMBOLS_NEED_BACKPOINTERS): Define if needed. * struc-symbol.h (SYMBOLS_NEED_BACKPOINTERS): Don't set. (struct symbol): Move bsym to make it clearly the first field. Remove TARGET_SYMBOL_FIELDS. (symbolS): Don't typedef. (struct broken_word): Remove. (N_TYPE_seg, seg_N_TYPE): Move to symbol.h. (SEGMENT_TO_SYMBOL_TYPE, N_REGISTER): Likewise. (symbol_clear_list_pointers): Likewise. (symbol_insert, symbol_remove): Likewise. (symbol_previous, symbol_append): Likewise. (verify_symbol_chain, verify_symbol_chain_2): Likewise. (struct local_symbol): Define. (local_symbol_converted_p, local_symbol_mark_converted): Define. (local_symbol_resolved_p, local_symbol_mark_resolved): Define. (local_symbol_get_frag, local_symbol_set_frag): Define. (local_symbol_get_real_symbol): Define. (local_symbol_set_real_symbol): Define. Define. * write.c (write_object_file): Call resolve_local_symbol_values. * config/obj-ecoff.h (OBJ_SYMFIELD_TYPE): Define. (TARGET_SYMBOL_FIELDS): Don't define. * config/obj-elf.h (OBJ_SYMFIELD_TYPE): Add local field. If ECOFF_DEBUGGING, add ECOFF fields. (ELF_TARGET_SYMBOL_FIELDS, TARGET_SYMBOL_FIELDS): Don't define. * config/obj-multi.h (struct elf_obj_sy): Add local field. If ECOFF_DEBUGGING, add ECOFF fields. (ELF_TARGET_SYMBOL_FIELDS, TARGET_SYMBOL_FIELDS): Don't define. (ECOFF_DEBUG_TARGET_SYMBOL_FIELDS): Don't define. * config/tc-mcore.h: Don't include struc-symbol.h. (TARGET_SYMBOL_FIELDS): Don't define. (struct mcore_tc_sy): Define. (TC_SYMFIELD_TYPE): Define. * Many files: Use symbolS instead of struct symbol. Use new accessor functions rather than referring to symbolS fields directly. * read.c (s_mri_common): Don't add in value of line_label. * config/tc-mips.c (md_apply_fix): Correct parenthesization when checking for SEC_LINK_ONCE. * config/tc-sh.h (sh_fix_adjustable): Declare. * app.c (input_buffer): New static variable. (app_push): Save saved_input in allocated buffer. (app_pop): Restored saved_input. (do_scrub_chars): Change get parameter to take char * and int as arguments. Change GET macro to pass input_buffer to get function. Don't save input into allocated buffer. * as.h (do_scrub_chars): Update declaration. * input-file.c (input_file_get): Change to take char * and int. Read data into passed in buffer. Remove static buffer. * read.c (scrub_from_string): Change to take char * and int. Copy data into passed in buffer. * hash.h: Neaten. Declare hash_traverse. * hash.c: Complete rewrite based on BFD hashing code. * gasp.c (chunksize): New variable. * macro.c (macro_expand_body): Call hash_jam with NULL rather than hash_delete. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 4:33 ` Ian Lance Taylor 2004-04-27 5:19 ` Zack Weinberg @ 2004-04-27 11:43 ` Richard Earnshaw 1 sibling, 0 replies; 41+ messages in thread From: Richard Earnshaw @ 2004-04-27 11:43 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: Zack Weinberg, hp, schwab, binutils, Philip Blundell On Tue, 2004-04-27 at 04:39, Ian Lance Taylor wrote: > You left > out the truly dreadful ARM .symver, in which '@' starts a comment at > any time--except when it appears in the operand of .symver. And, > repeating myself, MRI mode. I'd dearly like to deprecate use of '@' as a comment char, but it's not trivial. There simply is no single char in ASCII (with the possible exception of '`') that can be used as a substitute and that doesn't have other uses in the syntax. One possibility would be to adopt a 2-character sequence (such as '//'), and to that extent I notice that there is already code in the parser to handle this. However, there's still the legacy issue. Old versions of gcc generate @ in abundance, and there is substantial hand-written assembly code out there that does similarly. One way of addressing this might be to reject '@' in the EABI conforming modes -- it provides a relatively clean cut-off point. Can the selection of comment characters be made command-line option dependent? R. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: demand_empty_rest_of_line and ignore_rest_of_line 2004-04-27 1:32 ` Zack Weinberg 2004-04-27 2:02 ` Hans-Peter Nilsson 2004-04-27 2:35 ` Alan Modra @ 2004-04-27 2:47 ` Ian Lance Taylor 2 siblings, 0 replies; 41+ messages in thread From: Ian Lance Taylor @ 2004-04-27 2:47 UTC (permalink / raw) To: Zack Weinberg; +Cc: hp, schwab, binutils Zack Weinberg <zack@codesourcery.com> writes: > Uh, when I said "collapse the parsing logic together" I meant "think > of a way to get rid of the machine-dependent parsers to the maximum > extent possible." While thinking very carefully about how to make the > parser go blazingly fast *without* special tricks. I'm quite > confident this is possible. Assembly syntax - with or without extra > added whitespace - is not complicated, nor does it vary that much > across architectures supported by GAS. That turns out to be an oversimplification. Consider i386 prefixes, as Alan mentioned. Consider the d10v, for which the assembler programmer (or the compiler) may specify packing orderings between two instructions written on the same line. Consider m68k operand syntax, which is sufficiently complex that we wrote a bison parser to handle it. Let's not even discuss MRI compatibility mode. Remember that while the assembler normally handles compiler output, it must also handle hand-written assembler code which is much more free in how operands and expressions are used. Look at the current complexity of do_scrub_chars() today, when it doesn't have to worry about many of the details handled by the machine dependent parsers. Can these issues be solved? Yes, of course they can. Would the resulting parser be maintainable and blazingly fast? I very much doubt that both could be achieved simultaneously. Even if were achieved, would it still be faster to use some equivalent of #NO_APP mode? Almost certainly. Ian ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2004-04-27 10:56 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-03-17 16:36 demand_empty_rest_of_line and ignore_rest_of_line Nathan Sidwell 2004-03-17 16:43 ` Hans-Peter Nilsson 2004-03-17 16:49 ` Ian Lance Taylor 2004-03-18 10:29 ` Nathan Sidwell 2004-03-18 13:15 ` Ian Lance Taylor 2004-04-23 23:15 ` Andreas Schwab 2004-04-24 17:36 ` Hans-Peter Nilsson 2004-04-24 17:57 ` Andreas Schwab 2004-04-24 18:26 ` Hans-Peter Nilsson 2004-04-24 19:22 ` Andreas Schwab 2004-04-24 21:31 ` Hans-Peter Nilsson 2004-04-24 21:33 ` Andreas Schwab 2004-04-24 23:25 ` Hans-Peter Nilsson 2004-04-24 23:37 ` Andreas Schwab 2004-04-25 0:03 ` Zack Weinberg 2004-04-25 0:22 ` Hans-Peter Nilsson 2004-04-26 0:28 ` Zack Weinberg 2004-04-26 0:58 ` Hans-Peter Nilsson 2004-04-26 2:14 ` Hans-Peter Nilsson 2004-04-25 23:35 ` Ian Lance Taylor 2004-04-26 0:51 ` Zack Weinberg 2004-04-26 2:46 ` Ian Lance Taylor 2004-04-26 3:13 ` Zack Weinberg 2004-04-26 14:16 ` Ian Lance Taylor 2004-04-26 14:22 ` Andreas Schwab 2004-04-26 14:34 ` Richard Earnshaw 2004-04-26 15:29 ` Ian Lance Taylor 2004-04-26 19:26 ` Hans-Peter Nilsson 2004-04-26 19:42 ` Kai Henningsen 2004-04-26 19:45 ` Ian Lance Taylor 2004-04-26 20:04 ` Ian Lance Taylor 2004-04-27 1:32 ` Zack Weinberg 2004-04-27 2:02 ` Hans-Peter Nilsson 2004-04-27 2:38 ` Zack Weinberg 2004-04-27 2:35 ` Alan Modra 2004-04-27 3:13 ` Zack Weinberg 2004-04-27 4:33 ` Ian Lance Taylor 2004-04-27 5:19 ` Zack Weinberg 2004-04-27 6:52 ` Ian Lance Taylor 2004-04-27 11:43 ` Richard Earnshaw 2004-04-27 2:47 ` Ian Lance Taylor
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).