Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

[John David Anglin]

> On Sun, 24 Sep 2000, John David Anglin wrote:
> 
> > The fix for the problem is to change arg_reloc_stub_needed so that stubs
> > are generated for recursive calls.
> 
> This might cure the problem, but I don't think it's correct.  CALLEE !=
> CALLER in arg_reloc_stub_needed really means the callers idea of which
> registers to use to pass parameters and return values, differs from the
> actual registers used by the called function (which is why you need an
> arg reloc stub).

I shouldn't stay up so late trying to fix these things!  I think that
arg_reloc_stub_needed macro may not be doing the right thing and that
this has been hiding the problem to some extent.

When the function `int f(char *p)' is called recursively, I see the following
in md_apply_fix when the function is called:

Breakpoint 1, md_apply_fix (fixP=0x40026f10, valp=0x7b03ab84)
    at ./config/tc-hppa.c:4412
4412          if ((fmt == 12 || fmt == 17 || fmt == 22)
(gdb) p fixP->fx_pcrel
$3 = 1
(gdb) p (((obj_symbol_type *) symbol_get_bfdsym (fixP->fx_addsy))->tc_data.ap.hppa_arg_reloc)
$4 = 257
(gdb) p hppa_fixP->fx_arg_reloc
$5 = 256

It doesn't see right that a simple function such as the one above should
need an argument relocation stub.  If I change the return type to void, then
both values are 256 and no stub is needed.   I haven't been able to 
find where the bits for fx_arg_reloc are defined.  Does this seem right?

> I've been playing a little with the following, which isn't yet ready
> to install mainly because I noticed a few other things along the way, and
> it's gotten too late here for me to think straight..

It definitely looks like your on the right track here.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

[Alan Modra]

On Mon, 25 Sep 2000, John David Anglin wrote:

> I haven't been able to find where the bits for fx_arg_reloc are defined.

"man a.out" on an HP box.  Search for "parameter relocation"

-- 
Linuxcare.  Support for the Revolution.

Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

[Alan Modra]

On Tue, 26 Sep 2000, John David Anglin wrote:

> There is one test failure: reduce.s.  The new code no longer generates
> a R_PCREL_CALL call to foo.  However, I believe that this is correct

Fixed like this.

gas/testsuite/ChangeLog
	* gas/hppa/reloc/reduce.s: Modify .PARAM so we need an arg reloc.

Alan Modra
-- 
Linuxcare.  Support for the Revolution.

Index: gas/hppa/reloc/reduce.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/hppa/reloc/reduce.s,v
retrieving revision 1.2
diff -u -p -r1.2 reduce.s
--- reduce.s	1999/08/30 21:00:59	1.2
+++ reduce.s	2000/09/28 08:34:06
@@ -4,7 +4,7 @@
 
 	.code
 	.align 4
-	.PARAM foo,RTNVAL=GR
+	.PARAM foo,ARGW0=FR
 foo:
 	.PROC
 	.CALLINFO FRAME=0,NO_CALLS

Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

[John David Anglin]

> > A possible fix is attached below.  Note there is another small fix
> > to pa_align as well.
> 
> Thanks.  Your analysis looks correct.
> 
> Hmm, let's see
> 
> #define nonzero_dibits(x) \
>   ((x) | (((x) & 0x55555555) << 1) | (((x) & 0xAAAAAAAA) >> 1))
> #define arg_reloc_stub_needed(CALLER, CALLEE) \
>   (((CALLER) ^ (CALLEE)) & nonzero_dibits (CALLER) & nonzero_dibits (CALLEE))
> 
> should work too.

Pretty tricky.  I used a function rather than a macro because of the function
evaluation needed for the caller symbol_arg_reloc_info (fixP->fx_addsy).
However, I didn't look at optimized code so maybe the optimizer will
eliminate the multiple calls.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

[Alan Modra]

On Tue, 26 Sep 2000, John David Anglin wrote:

> Changed my mind about the problem being in gcc.  The default when an
> argument description is omitted from a .CALL statement is ARG = NO.
> NO means the argument cannot be relocated.  Thus, the definition of
> arg_reloc_stub_needed needs to be fixed to check all four args and the
> return separately.
> 
> A possible fix is attached below.  Note there is another small fix
> to pa_align as well.

Thanks.  Your analysis looks correct.

Hmm, let's see

#define nonzero_dibits(x) \
  ((x) | (((x) & 0x55555555) << 1) | (((x) & 0xAAAAAAAA) >> 1))
#define arg_reloc_stub_needed(CALLER, CALLEE) \
  (((CALLER) ^ (CALLEE)) & nonzero_dibits (CALLER) & nonzero_dibits (CALLEE))

should work too.

-- 
Linuxcare.  Support for the Revolution.

Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

[John David Anglin]

> > It doesn't see right that a simple function such as the one above should
> > need an argument relocation stub.  If I change the return type to void, then
> > both values are 256 and no stub is needed.   I haven't been able to 
> > find where the bits for fx_arg_reloc are defined.  Does this seem right?
> 
> There is a problem here but it doesn't appear to be with binutils.  Rather,
> it is with gcc.  The .EXPORT statement generated for the above function is:
> 
>         .EXPORT f,ENTRY,PRIV_LEV=3,ARGW0=GR,RTNVAL=GR
> 
> However, the .CALL generated doesn't include RTNVAL=GR:
> 
>         .CALL ARGW0=GR
> 
> Since the .CALL is missing a RTNVAL=GR, the two don't match and an argument
> relocation stub is being generated for all functions with a return in a GR.

Changed my mind about the problem being in gcc.  The default when an
argument description is omitted from a .CALL statement is ARG = NO.
NO means the argument cannot be relocated.  Thus, the definition of
arg_reloc_stub_needed needs to be fixed to check all four args and the
return separately.

A possible fix is attached below.  Note there is another small fix
to pa_align as well.

There is one test failure: reduce.s.  The new code no longer generates
a R_PCREL_CALL call to foo.  However, I believe that this is correct
since foo is `static' (ie., not exported) and an argument relocation
stub is not required for the call.

--- reduce.o.as.reloc	Tue Sep 26 16:15:18 2000
+++ reduce.o.as-new.reloc	Tue Sep 26 16:15:32 2000
@@ -1,5 +1,5 @@
 
-reduce.o.as:     file format som
+reduce.o.as-new:     file format som
 
 RELOCATION RECORDS FOR [$CODE$]:
 OFFSET   TYPE              VALUE 
@@ -9,7 +9,6 @@
 00000008 R_ENTRY           *ABS*+0x08000008
 00000008 R_CODE_ONE_SYMBOL  $CODE$+0x00000004
 0000000c R_CODE_ONE_SYMBOL  $CODE$+0x00000004
-00000014 R_PCREL_CALL      foo+0x40000000
 00000028 R_EXIT            *ABS*+0x00000010
 
Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2000-09-26  J. David Anglin  <dave@hiauly1.hia.nrc.ca>

	* config/tc-hppa.c (arg_reloc_stub_needed): Use
	pa_arg_reloc_stub_needed to check arguments and return separately.
	(pa_arg_reloc_stub_needed): New function.
	(arg_reloc_check): New macro.
	(pa_align): Declare argument `bytes'.

--- config/tc-hppa.c.alan	Mon Sep 25 15:24:54 2000
+++ config/tc-hppa.c	Tue Sep 26 15:27:28 2000
@@ -4323,8 +4323,24 @@
 }
 
 #if defined (OBJ_SOM) || defined (ELF_ARG_RELOC)
+#define arg_reloc_check(a, b, m) \
+  (((a) & (m)) && ((b) & (m)) && (((a) & (m)) != ((b) & (m))))
+
+/* Check all arguments and return value to see if caller and callee
+   agree on their location. */
+static int
+pa_arg_reloc_stub_needed (caller, callee)
+  unsigned int caller, callee;
+{
+  return (arg_reloc_check (caller, callee, 0x003)
+    || arg_reloc_check (caller, callee, 0x00c)
+    || arg_reloc_check (caller, callee, 0x030)
+    || arg_reloc_check (caller, callee, 0x0c0)
+    || arg_reloc_check (caller, callee, 0x300));
+}
+
 #define arg_reloc_stub_needed(CALLER, CALLEE) \
-  ((CALLEE) && (CALLER) && ((CALLEE) != (CALLER)))
+  pa_arg_reloc_stub_needed ((CALLER), (CALLEE))
 #else
 #define arg_reloc_stub_needed(CALLER, CALLEE) 0
 #endif
@@ -5831,6 +5847,7 @@
    alignment of the subspace if necessary.  */
 static void
 pa_align (bytes)
+  int bytes;
 {
   /* We must have a valid space and subspace.  */
   pa_check_current_space_and_subspace ();

Re: [Patch]: Re: gas 2.10.91 from 20000920 cvs is broken under hpux

[John David Anglin]

> When the function `int f(char *p)' is called recursively, I see the following
> in md_apply_fix when the function is called:
> 
> Breakpoint 1, md_apply_fix (fixP=0x40026f10, valp=0x7b03ab84)
>     at ./config/tc-hppa.c:4412
> 4412          if ((fmt == 12 || fmt == 17 || fmt == 22)
> (gdb) p fixP->fx_pcrel
> $3 = 1
> (gdb) p (((obj_symbol_type *) symbol_get_bfdsym (fixP->fx_addsy))->tc_data.ap.hppa_arg_reloc)
> $4 = 257
> (gdb) p hppa_fixP->fx_arg_reloc
> $5 = 256
> 
> It doesn't see right that a simple function such as the one above should
> need an argument relocation stub.  If I change the return type to void, then
> both values are 256 and no stub is needed.   I haven't been able to 
> find where the bits for fx_arg_reloc are defined.  Does this seem right?

There is a problem here but it doesn't appear to be with binutils.  Rather,
it is with gcc.  The .EXPORT statement generated for the above function is:

        .EXPORT f,ENTRY,PRIV_LEV=3,ARGW0=GR,RTNVAL=GR

However, the .CALL generated doesn't include RTNVAL=GR:

        .CALL ARGW0=GR

Since the .CALL is missing a RTNVAL=GR, the two don't match and an argument
relocation stub is being generated for all functions with a return in a GR.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)