public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [RFA:] Fix bug with #APP/#NO_APP when using macros.
@ 2003-05-08  8:08 Hans-Peter Nilsson
  2003-05-08 21:39 ` Ian Lance Taylor
  2003-05-13  1:09 ` Hans-Peter Nilsson
  0 siblings, 2 replies; 12+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-08  8:08 UTC (permalink / raw)
  To: binutils

The app4 test is the one closest to the original case, while the
others caught other bugs I made and stirred up during fixing.
Since the tests are all small and test a few corner cases, I
thought best to include them all.

Regarding the read.c patch, note that new_tmp isn't used after
the break, so old "new_tmp += size;" was dead code.  Not only
was the old_* kludge I replaced nauseating, it was also buggy;
it couldn't handle the input feed changing with the usual
mechanisms (now also used as per the patch), as when expanding a
macro.  The change is minimal in that I didn't rewrite the whole
#APP expansion thing (with its use of temporary strings), I just
made the #APP expansion go finally into a string buffer as used
elsewhere.

In response to the comment in the #APP processing ("The end of
the #APP wasn't in this buffer.  We keep reading in buffers
until we find the #NO_APP that goes with this #APP.  There is
one.  The specs guarentee it..." (sic)): There is no "spec" for
#APP/#NO_APP, AFAIK.  In particular, there is no guarantee that
there's ever a #NO_APP for every #APP.  Actually, no gcc port
defines ASM_FILE_END to emit #NO_APP, AFAICT, so most often
there is no last #NO_APP for the last asm in file scope.  Or
perhaps the missing last #NO_APP is a generic gcc bug.
Anyhoo...  As seen with the test app2, even if there's a
matching #NO_APP, things go wrong.

An alternative is to remove this support completely and ignore
#NO_APP/#APP; always scrub.  This is IMHO reasonable, since
apparently no other GCC port uses this feature.

Who is this JF, and why wasn't the nice string buffer functions
that SAC wrote used?  Or do I mix up the timeline?  (Reply in
private, if so inclined.)

End of rant.

Tested on cris-axis-elf, i686-pc-linux-gnu and
mmix-knuth-mmixware (where I see that lots of mmo ld tests are
broken, film at 11), no regressions from this patch.  The patch
fixes the test-cases on these targets.

Ok to commit?

gas/testsuite:
	* gas/macros/app1.s, gas/macros/app1.d, gas/macros/app2.s,
	gas/macros/app2.d, gas/macros/app3.s, gas/macros/app3.d,
	gas/macros/app4.s, gas/macros/app4b.s, gas/macros/app4.d: New
	tests.
	* gas/macros/macros.exp: Run them.

gas:
	* read.c (old_buffer, old_input, old_limit): Remove variables.
 	(read_a_source_file): Delete label contin.
	<handling #APP/#NO_APP>: Use an "sb" to push #APP expansion into
	input as with macros, instead of in separate old_* variables.
	Zero-terminate string being scrubbed.

--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app1.d	Thu May  8 08:22:16 2003
@@ -0,0 +1,4 @@
+#nm: -n
+#name: APP with macro without NO_APP
+0+ t a
+0+[1-f] t b
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app1.s	Wed May  7 18:22:42 2003
@@ -0,0 +1,10 @@
+#NO_APP
+ .text
+ .macro foo
+a:
+ .long 42
+ .endm
+#APP
+ foo
+b:
+ .long 56
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app2.d	Thu May  8 08:21:41 2003
@@ -0,0 +1,4 @@
+#nm: -n
+#name: APP with macro then NO_APP
+0+ t a
+0+[1-f] t b
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app2.s	Thu May  8 06:21:39 2003
@@ -0,0 +1,11 @@
+#NO_APP
+ .text
+ .macro foo
+a:
+ .long 42
+ .endm
+#APP
+ foo
+b:
+ .long 56
+#NO_APP
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app3.d	Thu May  8 08:22:58 2003
@@ -0,0 +1,5 @@
+#nm: -n
+#name: APP with macro then NO_APP then more code
+0+ t a
+0+[1-f] t b
+0+[2-f] t c
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app3.s	Thu May  8 06:29:01 2003
@@ -0,0 +1,13 @@
+#NO_APP
+ .text
+ .macro foo
+a:
+ .long 42
+ .endm
+#APP
+ foo
+b:
+ .long 56
+#NO_APP
+c:
+ .long 78
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app4.d	Thu May  8 08:33:47 2003
@@ -0,0 +1,6 @@
+#as: -I$srcdir/$subdir
+#nm: -n
+#name: included file with .if 0 wrapped in APP/NO_APP, no final NO_APP, macro in main file
+0+ t d
+0+[1-f] t a
+0+[2-f] t b
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app4.s	Thu May  8 08:12:36 2003
@@ -0,0 +1,9 @@
+ .text
+ .macro foo
+a:
+ .long 42
+ .endm
+ .include "app4b.s"
+ foo
+b:
+ .long 56
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ app4b.s	Thu May  8 08:32:21 2003
@@ -0,0 +1,10 @@
+#NO_APP
+d:
+ .long 21
+#APP
+ .if 0
+#NO_APP
+ .err
+x:
+#APP
+ .endif
Index: macros.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/macros/macros.exp,v
retrieving revision 1.13
diff -c -p -r1.13 macros.exp
*** macros.exp	18 Nov 2002 08:28:40 -0000	1.13
--- macros.exp	8 May 2003 06:44:41 -0000
*************** if { ![istarget hppa*-*-*] || [istarget 
*** 40,42 ****
--- 40,47 ----
      setup_xfail "*c4x*-*-*" "*c54x*-*"
      run_dump_test strings
  }
+ 
+ run_dump_test app1
+ run_dump_test app2
+ run_dump_test app3
+ run_dump_test app4

Index: read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.60
diff -c -p -r1.60 read.c
*** read.c	2 May 2003 02:41:45 -0000	1.60
--- read.c	8 May 2003 06:38:26 -0000
*************** static char *buffer_limit;	/*->1 + last 
*** 168,177 ****
     internals manual.  */
  int target_big_endian = TARGET_BYTES_BIG_ENDIAN;
  
- static char *old_buffer;	/* JF a hack.  */
- static char *old_input;
- static char *old_limit;
- 
  /* Variables for handling include file directory table.  */
  
  /* Table of pointers to directories to search for .include's.  */
--- 168,173 ----
*************** read_a_source_file (name)
*** 533,541 ****
    while ((buffer_limit = input_scrub_next_buffer (&input_line_pointer)) != 0)
      {				/* We have another line to parse.  */
        know (buffer_limit[-1] == '\n');	/* Must have a sentinel.  */
!     contin:			/* JF this goto is my fault I admit it.
! 				   Someone brave please re-write the whole
! 				   input section here?  Pleeze???  */
        while (input_line_pointer < buffer_limit)
  	{
  	  /* We have more of this buffer to parse.  */
--- 529,535 ----
    while ((buffer_limit = input_scrub_next_buffer (&input_line_pointer)) != 0)
      {				/* We have another line to parse.  */
        know (buffer_limit[-1] == '\n');	/* Must have a sentinel.  */
! 
        while (input_line_pointer < buffer_limit)
  	{
  	  /* We have more of this buffer to parse.  */
*************** read_a_source_file (name)
*** 953,958 ****
--- 947,953 ----
  
  	  if (c && strchr (line_comment_chars, c))
  	    {			/* Its a comment.  Better say APP or NO_APP.  */
+ 	      sb sbuf;
  	      char *ends;
  	      char *new_buf;
  	      char *new_tmp;
*************** read_a_source_file (name)
*** 965,970 ****
--- 960,966 ----
  		continue;	/* We ignore it */
  	      s += 4;
  
+ 	      sb_new (&sbuf);
  	      ends = strstr (s, "#NO_APP\n");
  
  	      if (!ends)
*************** read_a_source_file (name)
*** 1026,1032 ****
  
  		  if (size < space)
  		    {
! 		      new_tmp += size;
  		      break;
  		    }
  
--- 1022,1028 ----
  
  		  if (size < space)
  		    {
! 		      new_tmp[size] = 0;
  		      break;
  		    }
  
*************** read_a_source_file (name)
*** 1037,1049 ****
  
  	      if (tmp_buf)
  		free (tmp_buf);
- 	      old_buffer = buffer;
- 	      old_input = input_line_pointer;
- 	      old_limit = buffer_limit;
- 	      buffer = new_buf;
- 	      input_line_pointer = new_buf;
- 	      buffer_limit = new_tmp;
  
  	      continue;
  	    }
  
--- 1033,1051 ----
  
  	      if (tmp_buf)
  		free (tmp_buf);
  
+ 	      /* We've "scrubbed" input to the preferred format.  In the
+ 		 process we may have consumed the whole of the remaining
+ 		 file (and included files).  We handle this formatted
+ 		 input similar to that of macro expansion, letting
+ 		 actual macro expansion (possibly nested) and other
+ 		 input expansion work.  Beware that in messages, line
+ 		 numbers and possibly file names will be incorrect.  */
+ 	      sb_add_string (&sbuf, new_buf);
+ 	      input_scrub_include_sb (&sbuf, input_line_pointer, 0);
+ 	      sb_kill (&sbuf);
+ 	      buffer_limit = input_scrub_next_buffer (&input_line_pointer);
+ 	      free (new_buf);
  	      continue;
  	    }
  
*************** read_a_source_file (name)
*** 1061,1080 ****
  #ifdef md_after_pass_hook
        md_after_pass_hook ();
  #endif
- 
-       if (old_buffer)
- 	{
- 	  free (buffer);
- 	  bump_line_counters ();
- 	  if (old_input != 0)
- 	    {
- 	      buffer = old_buffer;
- 	      input_line_pointer = old_input;
- 	      buffer_limit = old_limit;
- 	      old_buffer = 0;
- 	      goto contin;
- 	    }
- 	}
      }
  
   quit:
--- 1063,1068 ----

brgds, H-P

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-08  8:08 [RFA:] Fix bug with #APP/#NO_APP when using macros Hans-Peter Nilsson
@ 2003-05-08 21:39 ` Ian Lance Taylor
  2003-05-09  1:09   ` Hans-Peter Nilsson
  2003-05-13  1:09 ` Hans-Peter Nilsson
  1 sibling, 1 reply; 12+ messages in thread
From: Ian Lance Taylor @ 2003-05-08 21:39 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: binutils

Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:

> An alternative is to remove this support completely and ignore
> #NO_APP/#APP; always scrub.  This is IMHO reasonable, since
> apparently no other GCC port uses this feature.

There are several gcc ports which currently use #NO_APP.  Grep for it
in gcc/config/*.h.  As I recall, #NO_APP was originally invented for
the m68k port.  Originally, gas was intended to only be used as the
backend of the compiler, and was not intended to support hand written
assembly code.  RMS objected to features like macros when I introduced
them, on the grounds that they would slow down the assembler, and also
that they would impose a future maintenance burden.

Of course, it would not be incorrect to ignore #NO_APP.  On the other
hand, #NO_APP was introduced to speed up the assembler, by skipping
the scrubbing.  On the gripping hand, scrubbing was a lot slower
before I rewrote the scrubber to use a state machine, back in 1995.

It would be interesting to profile the assembler to see how much time
would be lost by ignoring #NO_APP.  Try assembling the same file
twice, once with a patched assembler which always scrubs, and compare
the timings.  (I always used to use cccp.c for this sort of test case,
as a convenient very large source file; it is now gone, alas).

> Who is this JF, and why wasn't the nice string buffer functions
> that SAC wrote used?  Or do I mix up the timeline?  (Reply in
> private, if so inclined.)

JF is Jay Fenlason.  He was the gas maintainer up until 1991 or so.
You can still see his name in the docs, but the old ChangeLogs, such
as they were, have been lost.  That code in read.c is indeed at least
that old; a version of it is in my copy of gas 1.35 from March 1990.
The string buffer functions weren't written until SAC wrote gasp in
1994.  So there you go.

Ian

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-08 21:39 ` Ian Lance Taylor
@ 2003-05-09  1:09   ` Hans-Peter Nilsson
  2003-05-09  3:39     ` Ian Lance Taylor
  0 siblings, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-09  1:09 UTC (permalink / raw)
  To: ian; +Cc: hans-peter.nilsson, binutils

> From: Ian Lance Taylor <ian@airs.com>
> Date: 08 May 2003 14:38:55 -0700

> Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:
> 
> > An alternative is to remove this support completely and ignore
> > #NO_APP/#APP; always scrub.  This is IMHO reasonable, since
> > apparently no other GCC port uses this feature.
> 
> There are several gcc ports which currently use #NO_APP.  Grep for it
> in gcc/config/*.h.

I assume you mean ASM_APP_ON/OFF, emitted around inline asm.  If
so, that's my point and apparently a major misconception: If not
the very first thing in the file, #NO_APP (and #APP) are
ineffective and do not cause scrubbing of the file; strict
formatting is assumed.  Any later #APP and #NO_APP are then
actually scrubbed away (for reference .include:d files count as
separate).  To make #NO_APP the very first thing in the
GCC-emitted assembly-file, it has to be emitted by the port: it
should define ASM_FILE_START to do so.  GCC does not emit
ASM_APP_OFF in the file prologue by itself and hasn't done so in
the single-digit years I can remember.  Only cris-* defines
ASM_FILE_START so AFAICT, though some ports emit #NO_APP as the
second or third line(!).  For GCC, refer to
toplev.c:init_asm_output.  For gas, refer to
input-file.c:input_file_open.

Actually, if you change a port to emit #NO_APP first, you'll
notice for glibc systems that (as I muttered in a previous post)
glibc relies on a kludge based on scrubbing being done.  At
least it did last time I was there.  (Looking...) it seems it
still does.  See include/libc-symbols.h:link_warning: ``Tacking
on "\n\t#" to the section name makes gcc put it's bogus section
attributes on what looks like a comment to the assembler.''
Unfortunately no assembly comments are allowed with #NO_APP
(i.e. scrubbing not done), so you then get assembler syntax
errors.  For cris-axis-linux-gnu, glibc for cris-axis-linux-gnu
is compiled with -mpdebug, which is a GCC port-specific
assembly-output debug option with the side-effect of omitting
the first #NO_APP.  (For reference, this was in the original
glibc CRIS port submission, which should be in glibc ml archives
around 2001-04-08, but the option was misunderstood for some
reason and was omitted when sysdeps/cris/Makefile was committed,
thus breaking the port.  [Hmm, I really should revisit for this
and other reasons.  Oh well, I'll schedule that for later this
year.])

> It would be interesting to profile the assembler to see how much time
> would be lost by ignoring #NO_APP.  Try assembling the same file
> twice, once with a patched assembler which always scrubs, and compare
> the timings.  (I always used to use cccp.c for this sort of test case,
> as a convenient very large source file; it is now gone, alas).

Ok, I'll put it on my TODO list to time a build of a C-only
cris-* newlib-based toolchain with and without such a change.
It should be just as easy as compiling a single file, and C-only
is (still ;-) reasonably fast with GCC that assembly scrubbing
time should be a measurable, at least with that many files.

brgds, H-P

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-09  1:09   ` Hans-Peter Nilsson
@ 2003-05-09  3:39     ` Ian Lance Taylor
  2003-05-09  4:01       ` Zack Weinberg
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Lance Taylor @ 2003-05-09  3:39 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: binutils

Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:

> I assume you mean ASM_APP_ON/OFF, emitted around inline asm.  If
> so, that's my point and apparently a major misconception: If not
> the very first thing in the file, #NO_APP (and #APP) are
> ineffective and do not cause scrubbing of the file; strict
> formatting is assumed.  Any later #APP and #NO_APP are then
> actually scrubbed away (for reference .include:d files count as
> separate).  To make #NO_APP the very first thing in the
> GCC-emitted assembly-file, it has to be emitted by the port: it
> should define ASM_FILE_START to do so.  GCC does not emit
> ASM_APP_OFF in the file prologue by itself and hasn't done so in
> the single-digit years I can remember.  Only cris-* defines
> ASM_FILE_START so AFAICT, though some ports emit #NO_APP as the
> second or third line(!).  For GCC, refer to
> toplev.c:init_asm_output.  For gas, refer to
> input-file.c:input_file_open.

It's true that #NO_APP needs to be printed in ASM_FILE_START, but it's
not true that the CRIS target is the only target which does it.
m68k.h does it also.  I actually thought that the m68k was the only
target which used #NO_APP, but I see that you used it for the CRIS.

I take your point, though, which is that though targets other than
CRIS and m68k emit #APP/#NO_APP around assembler statements, it is
ineffective and pointless.

> Actually, if you change a port to emit #NO_APP first, you'll
> notice for glibc systems that (as I muttered in a previous post)
> glibc relies on a kludge based on scrubbing being done.  At
> least it did last time I was there.  (Looking...) it seems it
> still does.  See include/libc-symbols.h:link_warning: ``Tacking
> on "\n\t#" to the section name makes gcc put it's bogus section
> attributes on what looks like a comment to the assembler.''
> Unfortunately no assembly comments are allowed with #NO_APP
> (i.e. scrubbing not done), so you then get assembler syntax
> errors.  For cris-axis-linux-gnu, glibc for cris-axis-linux-gnu
> is compiled with -mpdebug, which is a GCC port-specific
> assembly-output debug option with the side-effect of omitting
> the first #NO_APP.  (For reference, this was in the original
> glibc CRIS port submission, which should be in glibc ml archives
> around 2001-04-08, but the option was misunderstood for some
> reason and was omitted when sysdeps/cris/Makefile was committed,
> thus breaking the port.  [Hmm, I really should revisit for this
> and other reasons.  Oh well, I'll schedule that for later this
> year.])

Probably nobody builds glibc for the m68k, since m68k Unix systems are
pretty rare these days.  Particularly m68k ELF systems.

Ian

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-09  3:39     ` Ian Lance Taylor
@ 2003-05-09  4:01       ` Zack Weinberg
  2003-05-09  4:12         ` Ian Lance Taylor
                           ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Zack Weinberg @ 2003-05-09  4:01 UTC (permalink / raw)
  To: binutils; +Cc: hans-peter.nilsson

Ian Lance Taylor <ian@airs.com> writes:

> It's true that #NO_APP needs to be printed in ASM_FILE_START, but it's
> not true that the CRIS target is the only target which does it.
> m68k.h does it also.  I actually thought that the m68k was the only
> target which used #NO_APP, but I see that you used it for the CRIS.
>
> I take your point, though, which is that though targets other than
> CRIS and m68k emit #APP/#NO_APP around assembler statements, it is
> ineffective and pointless.

And since no one has noticed this up to now, does that mean that there
is no real performance benefit to be had from #NO_APP, and we can blow
away all the code that emits it from gcc?  Or should gcc be changed to
emit #NO_APP at file start?

zw

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-09  4:01       ` Zack Weinberg
@ 2003-05-09  4:12         ` Ian Lance Taylor
  2003-05-09  4:38         ` Alan Modra
  2003-05-09  9:10         ` Hans-Peter Nilsson
  2 siblings, 0 replies; 12+ messages in thread
From: Ian Lance Taylor @ 2003-05-09  4:12 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: binutils, hans-peter.nilsson

Zack Weinberg <zack@codesourcery.com> writes:

> Ian Lance Taylor <ian@airs.com> writes:
> 
> > I take your point, though, which is that though targets other than
> > CRIS and m68k emit #APP/#NO_APP around assembler statements, it is
> > ineffective and pointless.
> 
> And since no one has noticed this up to now, does that mean that there
> is no real performance benefit to be had from #NO_APP, and we can blow
> away all the code that emits it from gcc?

I don't think we can conclude that.  There must be some performance
benefit from #NO_APP, since in that case gas does not have to call
do_scrub_chars().  However, the benefit may be insignificant.  We
would need to do some timing tests.

> Or should gcc be changed to emit #NO_APP at file start?

You can't do it blindly.  You have to make sure that the gcc output is
consistent with what the assembler expects when it does not scrub.
The assembler input must not contain any comments, and must not
contain any extra whitespace.  At least some gcc targets do not meet
these conditions.  For example, I happen to know that the MIPS backend
does not.

Ian

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-09  4:01       ` Zack Weinberg
  2003-05-09  4:12         ` Ian Lance Taylor
@ 2003-05-09  4:38         ` Alan Modra
  2003-05-09  9:10         ` Hans-Peter Nilsson
  2 siblings, 0 replies; 12+ messages in thread
From: Alan Modra @ 2003-05-09  4:38 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: binutils, hans-peter.nilsson

On Thu, May 08, 2003 at 09:01:14PM -0700, Zack Weinberg wrote:
> And since no one has noticed this up to now, does that mean that there
> is no real performance benefit to be had from #NO_APP, and we can blow
> away all the code that emits it from gcc?  Or should gcc be changed to
> emit #NO_APP at file start?

I just ran a current assembler (without HPAs latest patch) on a
powerpc-linux insn-attrtab.s which happens to be well over 6M in
size.

Without #NO_APP (I used #NO-APP :))
real    0m2.450s
user    0m2.370s
sys     0m0.090s

With #NO_APP
real    0m1.917s
user    0m1.850s
sys     0m0.060s

To put these numbers into context, gcc -g took nearly 20 seconds to
produce the .s file.  ie. the assembly time improvement gained by
adding #NO_APP represents about 2.6% of the total time to produce the
object file.  For a -g -O2 compile the improvement was only 0.86%.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-09  4:01       ` Zack Weinberg
  2003-05-09  4:12         ` Ian Lance Taylor
  2003-05-09  4:38         ` Alan Modra
@ 2003-05-09  9:10         ` Hans-Peter Nilsson
  2003-05-09  9:28           ` Hans-Peter Nilsson
  2 siblings, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-09  9:10 UTC (permalink / raw)
  To: zack; +Cc: binutils

> From: Zack Weinberg <zack@codesourcery.com>
> Date: Thu, 08 May 2003 21:01:14 -0700

> Ian Lance Taylor <ian@airs.com> writes:
> 
> > It's true that #NO_APP needs to be printed in ASM_FILE_START, but it's
> > not true that the CRIS target is the only target which does it.
> > m68k.h does it also.  I actually thought that the m68k was the only
> > target which used #NO_APP, but I see that you used it for the CRIS.

Right, I see now and should have looked closer at m68k.h (lots
of ASM_FILE_START in config/m68k/*) particularly since you
pointed out that port.  Sorry for the misinformation.  (Luckily
I used weasel phrasing.  Bother, it seems to prove I can't tell
very far! :-)

> > I take your point, though, which is that though targets other than
> > CRIS and m68k emit #APP/#NO_APP around assembler statements, it is
> > ineffective and pointless.
> 
> And since no one has noticed this up to now, does that mean that there
> is no real performance benefit to be had from #NO_APP, and we can blow
> away all the code that emits it from gcc?  Or should gcc be changed to
> emit #NO_APP at file start?

The latter, per port, with caution and after fixing glibc, since
Alan's measurements (thanks!) prove it's good enough (IMHO) to
use it.  Rare insn output templates in particular need
inspection for strict formatting.  Check the emitted trampoline
code too, or emit #APP/#NO_APP around it.

brgds, H-P

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-09  9:10         ` Hans-Peter Nilsson
@ 2003-05-09  9:28           ` Hans-Peter Nilsson
  0 siblings, 0 replies; 12+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-09  9:28 UTC (permalink / raw)
  To: binutils

> Date: Fri, 9 May 2003 11:10:23 +0200
> From: Hans-Peter Nilsson <hp@axis.com>

> > Ian Lance Taylor <ian@airs.com> writes:
> > 
> > > It's true that #NO_APP needs to be printed in ASM_FILE_START, but it's
> > > not true that the CRIS target is the only target which does it.
> > > m68k.h does it also.  I actually thought that the m68k was the only
> > > target which used #NO_APP, but I see that you used it for the CRIS.
> 
> Right, I see now and should have looked closer at m68k.h (lots
> of ASM_FILE_START in config/m68k/*) particularly since you
> pointed out that port.  Sorry for the misinformation.  (Luckily
> I used weasel phrasing.  Bother, it seems to prove I can't tell
> very far! :-)

(To try and make up for my failing, I investigated further.)
Even so, it seems most major m68k ELF ports in current GCC CVS
override the m68k.h ASM_FILE_START directly or indirectly (most
commonly by including elfos.h after m68k.h) with one that
doesn't emit #NO_APP.  See m68k*-*-netbsdelf* m68k-*-elf* and
m68k-*-linux*.  In effect, it's seems it's still possible that
the cris-* GCC port is the only one that emits #NO_APP at the
beginning of assembly files.  Whatever.

brgds, H-P

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-08  8:08 [RFA:] Fix bug with #APP/#NO_APP when using macros Hans-Peter Nilsson
  2003-05-08 21:39 ` Ian Lance Taylor
@ 2003-05-13  1:09 ` Hans-Peter Nilsson
  2003-05-13  1:27   ` Alan Modra
  1 sibling, 1 reply; 12+ messages in thread
From: Hans-Peter Nilsson @ 2003-05-13  1:09 UTC (permalink / raw)
  To: binutils

> Date: Thu, 8 May 2003 10:08:55 +0200
> From: Hans-Peter Nilsson <hp@axis.com>

Sorry to nag before even a week has passed.  Still, maybe a nag
is in order because it looks like the patch itself was forgotten
for the general #NO_APP issue, and the 2.14 release is in sight,
and the bug possibly affects future or modified Linux/CRIS
kernels: use a macro in arch/cris/kernel/entry.S after the
.include "entryoffsets.s" line and code after the macro
invocation is ignored.

> gas:
> 	* read.c (old_buffer, old_input, old_limit): Remove variables.
>  	(read_a_source_file): Delete label contin.
> 	<handling #APP/#NO_APP>: Use an "sb" to push #APP expansion into
> 	input as with macros, instead of in separate old_* variables.
> 	Zero-terminate string being scrubbed.

So, does this stand a chance to be reviewed and possibly approved
to be included in 2.14?  Thanks a huge amount for any answer.

brgds, H-P

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-13  1:09 ` Hans-Peter Nilsson
@ 2003-05-13  1:27   ` Alan Modra
  2003-05-13  2:39     ` Daniel Jacobowitz
  0 siblings, 1 reply; 12+ messages in thread
From: Alan Modra @ 2003-05-13  1:27 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: binutils

On Tue, May 13, 2003 at 03:09:11AM +0200, Hans-Peter Nilsson wrote:
> > gas:
> > 	* read.c (old_buffer, old_input, old_limit): Remove variables.
> >  	(read_a_source_file): Delete label contin.
> > 	<handling #APP/#NO_APP>: Use an "sb" to push #APP expansion into
> > 	input as with macros, instead of in separate old_* variables.
> > 	Zero-terminate string being scrubbed.
> 
> So, does this stand a chance to be reviewed and possibly approved
> to be included in 2.14?  Thanks a huge amount for any answer.

OK mainline.  I think it's OK for the branch too, give Daniel a
chance to disagree.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

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

* Re: [RFA:] Fix bug with #APP/#NO_APP when using macros.
  2003-05-13  1:27   ` Alan Modra
@ 2003-05-13  2:39     ` Daniel Jacobowitz
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Jacobowitz @ 2003-05-13  2:39 UTC (permalink / raw)
  To: Hans-Peter Nilsson, binutils

On Tue, May 13, 2003 at 10:57:27AM +0930, Alan Modra wrote:
> On Tue, May 13, 2003 at 03:09:11AM +0200, Hans-Peter Nilsson wrote:
> > > gas:
> > > 	* read.c (old_buffer, old_input, old_limit): Remove variables.
> > >  	(read_a_source_file): Delete label contin.
> > > 	<handling #APP/#NO_APP>: Use an "sb" to push #APP expansion into
> > > 	input as with macros, instead of in separate old_* variables.
> > > 	Zero-terminate string being scrubbed.
> > 
> > So, does this stand a chance to be reviewed and possibly approved
> > to be included in 2.14?  Thanks a huge amount for any answer.
> 
> OK mainline.  I think it's OK for the branch too, give Daniel a
> chance to disagree.

Go ahead for the branch also.  Thanks.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

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

end of thread, other threads:[~2003-05-13  2:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-05-08  8:08 [RFA:] Fix bug with #APP/#NO_APP when using macros Hans-Peter Nilsson
2003-05-08 21:39 ` Ian Lance Taylor
2003-05-09  1:09   ` Hans-Peter Nilsson
2003-05-09  3:39     ` Ian Lance Taylor
2003-05-09  4:01       ` Zack Weinberg
2003-05-09  4:12         ` Ian Lance Taylor
2003-05-09  4:38         ` Alan Modra
2003-05-09  9:10         ` Hans-Peter Nilsson
2003-05-09  9:28           ` Hans-Peter Nilsson
2003-05-13  1:09 ` Hans-Peter Nilsson
2003-05-13  1:27   ` Alan Modra
2003-05-13  2:39     ` Daniel Jacobowitz

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).