public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ia64 bundle alignment
@ 2005-01-24  9:51 Jan Beulich
  2005-01-28  5:05 ` James E Wilson
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2005-01-24  9:51 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 2659 bytes --]

With mixed code and data emission, bundles could be misaligned. While
there
appearantly was a previous attempt to address this (producing an error
in
such a case), the change below tries to get this in line with how the
Intel
assembler deals with this - forcing  the required alignment. Perhaps
the
previously added code would then be superfluous and could be removed.

Built and tested on ia64-unknown-linux-gnu.

Jan

gas/
2005-01-24  Jan Beulich  <jbeulich@novell.com>

	* config/tc-ia64.c (emit_one_bundle): Align fragment to 16
bytes.

gas/testsuite/
2005-01-24  Jan Beulich  <jbeulich@novell.com>

	* gas/ia64/balign[.ds]: New.
	* gas/ia64/ia64.exp: Run new test.

---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-18
10:43:33.000000000 +0100
+++ 2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-24
10:21:43.572025489 +0100
@@ -6191,6 +6191,7 @@ emit_one_bundle ()
   for (i = 0; i < 3; ++i)
     insn[i] = nop[ia64_templ_desc[template].exec_unit[i]];
 
+  frag_align (4, 0, 0);
   f = frag_more (16);
 
   /* Check to see if this bundle is at an offset that is a multiple of
16-bytes
---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/balign.d	1970-01-01
01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/balign.d	2005-01-21
15:42:17.000000000 +0100
@@ -0,0 +1,13 @@
+#objdump: -t
+#name: ia64 bundle align
+
+.*: +file format .*
+
+SYMBOL TABLE:
+0+00 l    d  .text[[:space:]]+[[:xdigit:]]+ .text
+0+00 l    d  .data[[:space:]]+[[:xdigit:]]+ .data
+0+00 l    d  .bss[[:space:]]+[[:xdigit:]]+ .bss
+0+00 l       .text[[:space:]]+[[:xdigit:]]+ info1
+0+40 l       .text[[:space:]]+[[:xdigit:]]+ info2
+0+10 g     F .text[[:space:]]+[[:xdigit:]]+ func1
+0+60 g     F .text[[:space:]]+[[:xdigit:]]+ func2
---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/balign.s	1970-01-01
01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/balign.s	2005-01-21
11:38:52.000000000 +0100
@@ -0,0 +1,18 @@
+.explicit
+.proc	func1
+info1:
+	data8	-1
+func1::
+	br.ret.sptk rp
+.endp	func1
+
+.align 0x40
+.proc	func2
+info2:
+	data8	-1
+	nop	0
+	nop	0
+	nop	0
+func2::
+	br.ret.sptk rp
+.endp	func2
---
/home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2004-07-02
08:26:34.000000000 +0200
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2005-01-24
10:21:43.573978614 +0100
@@ -46,6 +46,7 @@ if [istarget "ia64-*"] then {
 
     run_dump_test "real"
     run_dump_test "align"
+    run_dump_test "balign"
     run_dump_test "order"
     run_dump_test "global"
     if [istarget "ia64-*-hpux*"] then {


[-- Attachment #2: binutils-mainline-ia64-bundle-align.patch --]
[-- Type: text/plain, Size: 2658 bytes --]

With mixed code and data emission, bundles could be misaligned. While there
appearantly was a previous attempt to address this (producing an error in
such a case), the change below tries to get this in line with how the Intel
assembler deals with this - forcing  the required alignment. Perhaps the
previously added code would then be superfluous and could be removed.

Built and tested on ia64-unknown-linux-gnu.

Jan

gas/
2005-01-24  Jan Beulich  <jbeulich@novell.com>

	* config/tc-ia64.c (emit_one_bundle): Align fragment to 16 bytes.

gas/testsuite/
2005-01-24  Jan Beulich  <jbeulich@novell.com>

	* gas/ia64/balign[.ds]: New.
	* gas/ia64/ia64.exp: Run new test.

--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-18 10:43:33.000000000 +0100
+++ 2005-01-24.08.40/gas/config/tc-ia64.c	2005-01-24 10:21:43.572025489 +0100
@@ -6191,6 +6191,7 @@ emit_one_bundle ()
   for (i = 0; i < 3; ++i)
     insn[i] = nop[ia64_templ_desc[template].exec_unit[i]];
 
+  frag_align (4, 0, 0);
   f = frag_more (16);
 
   /* Check to see if this bundle is at an offset that is a multiple of 16-bytes
--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/balign.d	1970-01-01 01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/balign.d	2005-01-21 15:42:17.000000000 +0100
@@ -0,0 +1,13 @@
+#objdump: -t
+#name: ia64 bundle align
+
+.*: +file format .*
+
+SYMBOL TABLE:
+0+00 l    d  .text[[:space:]]+[[:xdigit:]]+ .text
+0+00 l    d  .data[[:space:]]+[[:xdigit:]]+ .data
+0+00 l    d  .bss[[:space:]]+[[:xdigit:]]+ .bss
+0+00 l       .text[[:space:]]+[[:xdigit:]]+ info1
+0+40 l       .text[[:space:]]+[[:xdigit:]]+ info2
+0+10 g     F .text[[:space:]]+[[:xdigit:]]+ func1
+0+60 g     F .text[[:space:]]+[[:xdigit:]]+ func2
--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/balign.s	1970-01-01 01:00:00.000000000 +0100
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/balign.s	2005-01-21 11:38:52.000000000 +0100
@@ -0,0 +1,18 @@
+.explicit
+.proc	func1
+info1:
+	data8	-1
+func1::
+	br.ret.sptk rp
+.endp	func1
+
+.align 0x40
+.proc	func2
+info2:
+	data8	-1
+	nop	0
+	nop	0
+	nop	0
+func2::
+	br.ret.sptk rp
+.endp	func2
--- /home/jbeulich/src/binutils/mainline/2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2004-07-02 08:26:34.000000000 +0200
+++ 2005-01-24.08.40/gas/testsuite/gas/ia64/ia64.exp	2005-01-24 10:21:43.573978614 +0100
@@ -46,6 +46,7 @@ if [istarget "ia64-*"] then {
 
     run_dump_test "real"
     run_dump_test "align"
+    run_dump_test "balign"
     run_dump_test "order"
     run_dump_test "global"
     if [istarget "ia64-*-hpux*"] then {

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

* Re: [PATCH] ia64 bundle alignment
  2005-01-24  9:51 [PATCH] ia64 bundle alignment Jan Beulich
@ 2005-01-28  5:05 ` James E Wilson
  0 siblings, 0 replies; 3+ messages in thread
From: James E Wilson @ 2005-01-28  5:05 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils

On Mon, 2005-01-24 at 01:51, Jan Beulich wrote:
> With mixed code and data emission, bundles could be misaligned. While
> there
> appearantly was a previous attempt to address this (producing an error
> in
> such a case)

I left this one for last because I don't like it.  You shouldn't be
mixing code and data in ia64 assembly language.  It just causes too much
trouble.  It can be made to work in some restricted cases if you know
what you are doing, but not in general, and hence I don't think we
should be encouraging it by making it easier.  If anything, we should be
emitting more warnings for your example.  IMHO, you shouldn't be putting
data between proc and endp.  The data isn't part of the function.

It is trivial to make your examples work by adding .align directives to
your code.  I think that is the right solution in this case.

I understand that the Intel assembler is more permissive than GNU as
here, but the Intel assembler is broken.  It will accept some stuff that
can't possibly work, and then produce bad object files for you without
giving you any warning or error.  I don't want GNU as to do that.

I'd suggest using xdata directives instead of data directives.  This
allows you to put the data directives within a function, without
actually mixing code and data.

Also, don't ever use .section directives inside a function.  This breaks
DV checking.  We try to make it work by emitting padding nops and a stop
bit, but if you need serialization, you are screwed.  This is so
difficult to get right that I don't think we should even be trying. 
Instead, I think we should be discouraging section switches inside
functions, and similarly emitting data inside functions.

Besides the question of whether we should support this, there is also an
efficiency issue.  Calling frag_align will create a variant frag.  This
means we will end up with two frags for every instruction bundle.  This
will force the assembler to do a lot more work than it currently does,
just to support something people shouldn't be doing in the first place. 
This doesn't make much sense.  I think it makes more sense to ask you to
add .align directives to your code.

Also, historically, we have had problems with variant frags.  HJ fixed
the unwind info generation last year.  The generation of the unwind info
has to be delayed until after relaxation.  Before this was done, we
would get incorrect unwind info when variant frags for alignment were
present.  There could also be other hidden problems.  I'd prefer to
avoid generating unnecessary variant frags.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com


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

* Re: [PATCH] ia64 bundle alignment
@ 2005-01-28  8:04 Jan Beulich
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2005-01-28  8:04 UTC (permalink / raw)
  To: wilson; +Cc: binutils

OK, I can accept that. I just tried to get this in line with the Intel
assembler; I didn't intend to put data in a function myself... Jan

>>> James E Wilson <wilson@specifixinc.com> 28.01.05 06:04:55 >>>
On Mon, 2005-01-24 at 01:51, Jan Beulich wrote:
> With mixed code and data emission, bundles could be misaligned.
While
> there
> appearantly was a previous attempt to address this (producing an
error
> in
> such a case)

I left this one for last because I don't like it.  You shouldn't be
mixing code and data in ia64 assembly language.  It just causes too
much
trouble.  It can be made to work in some restricted cases if you know
what you are doing, but not in general, and hence I don't think we
should be encouraging it by making it easier.  If anything, we should
be
emitting more warnings for your example.  IMHO, you shouldn't be
putting
data between proc and endp.  The data isn't part of the function.

It is trivial to make your examples work by adding .align directives
to
your code.  I think that is the right solution in this case.

I understand that the Intel assembler is more permissive than GNU as
here, but the Intel assembler is broken.  It will accept some stuff
that
can't possibly work, and then produce bad object files for you without
giving you any warning or error.  I don't want GNU as to do that.

I'd suggest using xdata directives instead of data directives.  This
allows you to put the data directives within a function, without
actually mixing code and data.

Also, don't ever use .section directives inside a function.  This
breaks
DV checking.  We try to make it work by emitting padding nops and a
stop
bit, but if you need serialization, you are screwed.  This is so
difficult to get right that I don't think we should even be trying. 
Instead, I think we should be discouraging section switches inside
functions, and similarly emitting data inside functions.

Besides the question of whether we should support this, there is also
an
efficiency issue.  Calling frag_align will create a variant frag. 
This
means we will end up with two frags for every instruction bundle. 
This
will force the assembler to do a lot more work than it currently does,
just to support something people shouldn't be doing in the first place.

This doesn't make much sense.  I think it makes more sense to ask you
to
add .align directives to your code.

Also, historically, we have had problems with variant frags.  HJ fixed
the unwind info generation last year.  The generation of the unwind
info
has to be delayed until after relaxation.  Before this was done, we
would get incorrect unwind info when variant frags for alignment were
present.  There could also be other hidden problems.  I'd prefer to
avoid generating unnecessary variant frags.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com 


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

end of thread, other threads:[~2005-01-28  8:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-24  9:51 [PATCH] ia64 bundle alignment Jan Beulich
2005-01-28  5:05 ` James E Wilson
2005-01-28  8:04 Jan Beulich

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