Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

> What is the status of all this?  Does anyone now know the right way to
> proceed to fix this?

I believe that we are converging on a solution. :-)

A recent bootstrap succeeded with only my change to UNALIGNED_DOUBLE_INT_ASM_OP
and without the changes to the dw2_asm_output_* routines so it appears
changing those routines isn't necessary (which Richard rightly pointed out).
I do believe (and Richard okayed) that unaligned_integer_asm_op should abort
rather then return NULL.  I'll post an updated version of my patch tonight /
tomorrow.

The issue of replacing unaligned_integer_asm_op with assemble_integer isn't
that important to me as long as the compiler works, though there is some
overlap between these two functions.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Brad Lucier]

> 
> >On Thu, Sep 27, 2001 at 06:53:11PM -0400, John Wehle wrote:
> >> Hmm ... walking:
> >> 
> >>   assemble_integer (gen_rtx_MINUS (Pmode, gen_rtx_SYMBOL_REF (Pmode, lab1),
> >>                                    gen_rtx_SYMBOL_REF (Pmode, lab2)),
> >>                     size, BITS_PER_UNIT, 1);
> >> 
> >> I see that assemble_integer will output output ".uaword"...
> >
> >Consider if UNALIGNED_INT_ASM_OP is not defined.
> 
> If UNALIGNED_INT_ASM_OP is not defined then unaligned_integer_asm_op
> doesn't work in addition to assemble_integer not working.
> 
> So (ignoring possible whitespace formatting issues) assemble_integer
> appears capable of emitting things like:
> 
>   .uaword .L0 - .L1
> 
> and (at least for dw2_asm_output_delta) to do the same thing as:
> 
>   fputs (unaligned_integer_asm_op (size), asm_out_file);
>   assemble_name (asm_out_file, lab1);
>   fputc ('-', asm_out_file);
>   assemble_name (asm_out_file, lab2);
> 
> Perhaps wasn't true at one time, however it appears to be true now.
> 
> -- John

What is the status of all this?  Does anyone now know the right way to
proceed to fix this?

Brad

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

>On Thu, Sep 27, 2001 at 06:53:11PM -0400, John Wehle wrote:
>> Hmm ... walking:
>> 
>>   assemble_integer (gen_rtx_MINUS (Pmode, gen_rtx_SYMBOL_REF (Pmode, lab1),
>>                                    gen_rtx_SYMBOL_REF (Pmode, lab2)),
>>                     size, BITS_PER_UNIT, 1);
>> 
>> I see that assemble_integer will output output ".uaword"...
>
>Consider if UNALIGNED_INT_ASM_OP is not defined.

If UNALIGNED_INT_ASM_OP is not defined then unaligned_integer_asm_op
doesn't work in addition to assemble_integer not working.

So (ignoring possible whitespace formatting issues) assemble_integer
appears capable of emitting things like:

  .uaword .L0 - .L1

and (at least for dw2_asm_output_delta) to do the same thing as:

  fputs (unaligned_integer_asm_op (size), asm_out_file);
  assemble_name (asm_out_file, lab1);
  fputc ('-', asm_out_file);
  assemble_name (asm_out_file, lab2);

Perhaps wasn't true at one time, however it appears to be true now.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Kenner]

    > However unaligned_integer_asm_op doesn't which makes
    > UNALIGNED_DOUBLE_INT_ASM_OP being null dangerous.
    
    I don't see the difference between abort and SEGV, myself.

Not all machines SEGV for null dereference, so testing for it is safer.

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Henderson]

On Thu, Sep 27, 2001 at 07:50:07PM -0400, John Wehle wrote:
> I find an abort in unaligned_integer_asm_op a little more obvious.

Ok.


r~

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

> On Thu, Sep 27, 2001 at 07:05:29PM -0400, John Wehle wrote:
> > However unaligned_integer_asm_op doesn't which makes
> > UNALIGNED_DOUBLE_INT_ASM_OP being null dangerous.
> 
> I don't see the difference between abort and SEGV, myself.

The abort tells you exactly where the problem is.  Otherwise
a NULL is returned which is eventially passed to some other
routine which blows up.  The person debugging the problem
has to do more work to determine where things went wrong.
In this example fputs will probably go boom ... is there
a core generated, can the person run a debugger on the core,
can the debugger provide a reasonable backtrace, etc.  I
find an abort in unaligned_integer_asm_op a little more
obvious.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Henderson]

On Thu, Sep 27, 2001 at 07:05:29PM -0400, John Wehle wrote:
> However unaligned_integer_asm_op doesn't which makes
> UNALIGNED_DOUBLE_INT_ASM_OP being null dangerous.

I don't see the difference between abort and SEGV, myself.


r~

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Henderson]

On Thu, Sep 27, 2001 at 06:53:11PM -0400, John Wehle wrote:
> Hmm ... walking:
> 
>   assemble_integer (gen_rtx_MINUS (Pmode, gen_rtx_SYMBOL_REF (Pmode, lab1),
>                                    gen_rtx_SYMBOL_REF (Pmode, lab2)),
>                     size, BITS_PER_UNIT, 1);
> 
> I see that assemble_integer will output output ".uaword"...

Consider if UNALIGNED_INT_ASM_OP is not defined.


r~

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

> Yes, and assemble_integer knows how to deal with
> UNALIGNED_DOUBLE_INT_ASM_OP being null.

However unaligned_integer_asm_op doesn't which makes
UNALIGNED_DOUBLE_INT_ASM_OP being null dangerous.
At the very least unaligned_integer_asm_op should read:

  const char *op = NULL
  switch (size)
    {
    ...
#ifdef UNALIGNED_DOUBLE_INT_ASM_OP
      op = UNALIGNED_DOUBLE_INT_ASM_OP;
      break;
#endif
    default:
      abort ();
    }
  if (! op)
    abort ();
  return op;

so that it aborts instead of returning NULL.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

> > Doesn't imply that assemble_integer handles the very issue you're
> > raising (whitespace formatting issues aside)?
> 
> No, that's just left-over wishful thinking.

Hmm ... walking:

  assemble_integer (gen_rtx_MINUS (Pmode, gen_rtx_SYMBOL_REF (Pmode, lab1),
                                   gen_rtx_SYMBOL_REF (Pmode, lab2)),
                    size, BITS_PER_UNIT, 1);

I see that assemble_integer will output output ".uaword" because
align < size * BITS_PER_UNIT.  It then calls output_addr_const
which sees the MINUS so it calls itself to handle the operand0
which is a SYMBOL_REF so assemble_name is called. "-" is then
printed and output_addr_const is called again to handle operand1
which is another SYMBOL_REF so assemble_name is called.

this seems identical to:

  fputs (unaligned_integer_asm_op (size), asm_out_file);
  assemble_name (asm_out_file, lab1);
  fputc ('-', asm_out_file);
  assemble_name (asm_out_file, lab2);

I don't mean to be thick ... am I reading the code wrong?

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Henderson]

On Thu, Sep 27, 2001 at 06:26:33PM -0400, John Wehle wrote:
> Doesn't imply that assemble_integer handles the very issue you're
> raising (whitespace formatting issues aside)?

No, that's just left-over wishful thinking.  Also see for example:

#ifdef UNALIGNED_INT_ASM_OP
  fputs (unaligned_integer_asm_op (size), asm_out_file);
  assemble_name (asm_out_file, label);
  fputc ('-', asm_out_file);
  fputc ('.', asm_out_file);
#else
  abort ();
#endif


r~

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

> In general, it is not possible to replace dw2_asm_output_* with
> assemble_integer, because we want to do things like emit
> 
> 	.uaword	.L0 - .L1
> 
> which you cannot do in assemble_integer.

Umm ... dw2_asm_output_delta already has:

#ifdef UNALIGNED_INT_ASM_OP
  fputs (unaligned_integer_asm_op (size), asm_out_file);
  assemble_name (asm_out_file, lab1);
  fputc ('-', asm_out_file);
  assemble_name (asm_out_file, lab2);
#else
  assemble_integer (gen_rtx_MINUS (Pmode, gen_rtx_SYMBOL_REF (Pmode, lab1),
                                   gen_rtx_SYMBOL_REF (Pmode, lab2)),
                    size, BITS_PER_UNIT, 1);
#endif

Doesn't imply that assemble_integer handles the very issue you're
raising (whitespace formatting issues aside)?  BTW, I wasn't talking
about replacing dw2_asm_output_* with assemble_integer, merely
replacing uses of unaligned_integer_asm_op with assemble_integer.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Henderson]

On Thu, Sep 27, 2001 at 05:58:07PM -0400, John Wehle wrote:
> which leads me to believe that problem compiling linking.m isn't
> due to debug information.

Yes.

> UNALIGNED_DOUBLE_INT_ASM_OP is also
> used by assemble_integer in addition to dw2_asm_output_* so it is
> used anytime an unaligned double int is assembled.

Yes, and assemble_integer knows how to deal with
UNALIGNED_DOUBLE_INT_ASM_OP being null.

> I believed merely changing UNALIGNED_DOUBLE_INT_ASM_OP resulted
> in the bootstrap failing at a different spot, however I don't
> have the details at head.

Would you please find out?

> Perhaps assemble_integer can be improved to provide prettier output?
> It doesn't seem desirable to have different routines to do the same
> thing.

In general, it is not possible to replace dw2_asm_output_* with
assemble_integer, because we want to do things like emit

	.uaword	.L0 - .L1

which you cannot do in assemble_integer.  In fact, only one of the
N uses of unaligned_integer_asm_op in dwarf2asm.c is an integer
constant that could be broken down into pieces to emit.


r~

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

>On Thu, Sep 27, 2001 at 04:53:35PM -0400, John Wehle wrote:
>> When compiling libobjc/linking.m the compiler outputs ".uaxword" in
>> order to output a double int.
>
> But why?  I wouldn't have thought there would be any any double ints
> in debug info in 32-bit mode.

The traceback (with my patch installed) is:

#0  assemble_integer (x=0x39a400, size=8, align=32, force=0)
    at ../../../../../src/GNU/gcc-current/gcc/varasm.c:1966
#1  0x21d3c8 in output_constant (exp=0x4aa6c0, size=8, align=32)
    at ../../../../../src/GNU/gcc-current/gcc/varasm.c:4487
#2  0x21e380 in output_constructor (exp=0x4242d0, size=24, align=32)
    at ../../../../../src/GNU/gcc-current/gcc/varasm.c:4751
#3  0x21d520 in output_constant (exp=0x478140, size=24, align=32)
    at ../../../../../src/GNU/gcc-current/gcc/varasm.c:4535
#4  0x1fe420 in rest_of_decl_compilation (decl=0x424348, asmspec=0x0, 
    top_level=1, at_end=0)
    at ../../../../../src/GNU/gcc-current/gcc/toplev.c:2535
#5  0x55844 in finish_decl (decl=0x424348, init=0x35ec00, 
    asmspec_tree=0x32b0a8)
    at ../../../../../src/GNU/gcc-current/gcc/c-decl.c:3627
#6  0x3aa28 in build_selector_translation_table ()
    at ../../../../../src/GNU/gcc-current/gcc/objc/objc-act.c:2238

which leads me to believe that problem compiling linking.m isn't
due to debug information.  UNALIGNED_DOUBLE_INT_ASM_OP is also
used by assemble_integer in addition to dw2_asm_output_* so it is
used anytime an unaligned double int is assembled.

I believed merely changing UNALIGNED_DOUBLE_INT_ASM_OP resulted
in the bootstrap failing at a different spot, however I don't
have the details at head.  I may have simply tried undefining it
and didn't try (TARGET_ARCH64 ? "\t.uaxword\t" : NULL) without
the changes to dwarf2asm.c.  I'll revisit this later today / tomorrow.

> Yes, and makes -dA output really ugly.

Perhaps assemble_integer can be improved to provide prettier output?
It doesn't seem desirable to have different routines to do the same
thing.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Henderson]

On Thu, Sep 27, 2001 at 04:53:35PM -0400, John Wehle wrote:
> When compiling libobjc/linking.m the compiler outputs ".uaxword" in
> order to output a double int.

But why?  I wouldn't have thought there would be any any double ints
in debug info in 32-bit mode.

> The patch replaces unaligned_integer_asm_op with assemble_integer.

Yes, and makes -dA output really ugly.


r~

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[John Wehle]

>>   due to gcc outputing .uaxword which the Sun assembler dislikes when
>>   targeting the 32 bit instruction set.
>
>I suspect that one can simply do
>
>#define UNALIGNED_DOUBLE_INT_ASM_OP  (TARGET_ARCH64 ? ".uaxword" : NULL)

Unfortunately that doesn't suffice.

> but the patch didn't describe what caused the problem.

When compiling libobjc/linking.m the compiler outputs ".uaxword" in
order to output a double int.  Changing UNALIGNED_DOUBLE_INT_ASM_OP
to return NULL when ! TARGET_ARCH64 causes the bootstrap to fail
because the dw2_asm_output_* routines assume that unaligned_integer_asm_op
always returns a string.

assemble_integer is capable of outputting unaligned integers and
knows how to output a double int even when UNALIGNED_DOUBLE_INT_ASM_OP
is NULL.  The patch replaces unaligned_integer_asm_op with assemble_integer.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Re: Unreviewed patch (was: another bootstrap failure onsparc-sun-solaris28 in cp/xref.o)

[Richard Henderson]

On Thu, Sep 27, 2001 at 09:41:01AM +0200, Gerald Pfeifer wrote:
> > http://gcc.gnu.org/ml/gcc-patches/2001-09/msg00488.html
> >
> > which has not yet been reviewed to my knowledge.

Oh yes.  I keep meaning to look at this one in detail.

>   due to gcc outputing .uaxword which the Sun assembler dislikes when
>   targeting the 32 bit instruction set.

I suspect that one can simply do

#define UNALIGNED_DOUBLE_INT_ASM_OP  (TARGET_ARCH64 ? ".uaxword" : NULL)

but the patch didn't describe what caused the problem.


r~