public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* A new alpha bug
@ 1997-09-12 15:31 H.J. Lu
  1997-09-12 18:08 ` Richard Henderson
  1997-09-15 20:26 ` Jim Wilson
  0 siblings, 2 replies; 18+ messages in thread
From: H.J. Lu @ 1997-09-12 15:31 UTC (permalink / raw)
  To: egcs; +Cc: rth

Using egcs 970910 configured for linux/alpha, this code failed
to run on alpha with any optimization:

# gcc foo.cc
# a.out
# gcc -O foo.cc
# a.out
Floating point exception (core dumped)

gcc 2.7.2.1 for linux/alpha is fine.


-- 
H.J. Lu (hjl@gnu.ai.mit.edu)
---
class complex_double
{
public:
  complex_double (double r = 0, double i = 0): re (r), im (i) { }
  double re, im;
};

main()
{
  complex_double one = 1.0;
  if (one.re != 1.0 || one.im != 0.0)
    abort ();

  return 0;
}

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

* Re: A new alpha bug
  1997-09-12 15:31 A new alpha bug H.J. Lu
@ 1997-09-12 18:08 ` Richard Henderson
  1997-09-12 18:25   ` H.J. Lu
                     ` (2 more replies)
  1997-09-15 20:26 ` Jim Wilson
  1 sibling, 3 replies; 18+ messages in thread
From: Richard Henderson @ 1997-09-12 18:08 UTC (permalink / raw)
  To: H.J. Lu; +Cc: egcs

>   complex_double (double r = 0, double i = 0): re (r), im (i) { }
[...]
>   complex_double one = 1.0;

I don't think this does what you think it does.  What it doesn't
do is call the constructor listed. 

I'll leave it to those who can actually remember what the rules
concerning that equals sign are to determine if this is a bug in
the C++ front end or whether your code is wrong.


r~

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

* Re: A new alpha bug
  1997-09-12 18:08 ` Richard Henderson
@ 1997-09-12 18:25   ` H.J. Lu
       [not found]   ` <m0x9gxl-0004ecC.cygnus.egcs@ocean.lucon.org>
  1997-09-13 14:44   ` A new " H.J. Lu
  2 siblings, 0 replies; 18+ messages in thread
From: H.J. Lu @ 1997-09-12 18:25 UTC (permalink / raw)
  To: rth; +Cc: egcs

> 
> >   complex_double (double r = 0, double i = 0): re (r), im (i) { }
> [...]
> >   complex_double one = 1.0;
> 
> I don't think this does what you think it does.  What it doesn't
> do is call the constructor listed. 
> 
> I'll leave it to those who can actually remember what the rules
> concerning that equals sign are to determine if this is a bug in
> the C++ front end or whether your code is wrong.
> 

That example is taken from tcomplex.cc in libstdc++/tests.
It looks ok to me.

complex_double one = 1.0;

should be translate by compiler to

complex_double one;
complex_double tmp (1.0);
one = tmp;

That code works on x86. From what I have seen on the egcs list, that
also works on MIPS and Sparc. From what I saw in rtl, I think it may
be a combination of the C++ front end alpha back end.


-- 
H.J. Lu (hjl@gnu.ai.mit.edu)

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

* Re: A new alpha bug
       [not found]   ` <m0x9gxl-0004ecC.cygnus.egcs@ocean.lucon.org>
@ 1997-09-12 19:10     ` Jason Merrill
  1997-09-12 22:03       ` H.J. Lu
                         ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Jason Merrill @ 1997-09-12 19:10 UTC (permalink / raw)
  To: egcs

>>>>> H J Lu <hjl@lucon.org> writes:

>> >   complex_double (double r = 0, double i = 0): re (r), im (i) { }
>> [...]
>> >   complex_double one = 1.0;
>> 
>> I don't think this does what you think it does.  What it doesn't
>> do is call the constructor listed. 

Yes, it does.

> complex_double one = 1.0;

> should be translate by compiler to

> complex_double one;
> complex_double tmp (1.0);
> one = tmp;

Not quite.  More like

complex_double tmp (1.0, 0);
complex_double one (tmp);

The '=' in a declaration does not indicate use of the assignment operator.

Jason

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

* Re: A new alpha bug
  1997-09-12 19:10     ` Jason Merrill
@ 1997-09-12 22:03       ` H.J. Lu
  1997-09-14 11:40       ` H.J. Lu
  1997-09-14 11:56       ` More info on the g++ " H.J. Lu
  2 siblings, 0 replies; 18+ messages in thread
From: H.J. Lu @ 1997-09-12 22:03 UTC (permalink / raw)
  To: Jason Merrill; +Cc: egcs, rth

> 
> Not quite.  More like
> 
> complex_double tmp (1.0, 0);
> complex_double one (tmp);
> 
> The '=' in a declaration does not indicate use of the assignment operator.
> 

Of course, you are right. After comparing the foo.cc.rtl files from
gcc 2.7.2.1 -O, egcs 970910 and egcs 970910 -O, I think either
TImode in alpha and/or aliase for TImode are broken. Here is
foo.cc.rtl generated by egcs 970910 -O. As you can see, TI 69
is used without setting first.


-- 
H.J. Lu (hjl@gnu.ai.mit.edu)
---foo.cc.rtl---
;; Function complex_double::complex_double(double = 0, double = 0)

(note 2 0 3 "" NOTE_INSN_DELETED)

(note 3 2 5 "" NOTE_INSN_DELETED)

(insn 5 3 7 (set (reg/v/u:DI 69)
        (reg:DI 16 $16)) -1 (nil)
    (nil))

(insn 7 5 9 (set (reg/v:DF 70)
        (reg:DF 49 $f17)) -1 (nil)
    (nil))

(insn 9 7 10 (set (reg/v:DF 71)
        (reg:DF 50 $f18)) -1 (nil)
    (nil))

(note 10 9 11 "" NOTE_INSN_FUNCTION_BEG)

(note 11 10 13 "" NOTE_INSN_DELETED)

(note 13 11 14 "" NOTE_INSN_DELETED)

(note 14 13 16 "" NOTE_INSN_DELETED)

(insn 16 14 17 (set (mem/s:DF (reg/v/u:DI 69))
        (reg/v:DF 70)) -1 (nil)
    (nil))

(note 17 16 19 "" NOTE_INSN_DELETED)

(insn 19 17 12 (set (mem/s:DF (plus:DI (reg/v/u:DI 69)
                (const_int 8)))
        (reg/v:DF 71)) -1 (nil)
    (nil))

(note 12 19 21 "" NOTE_INSN_DELETED)

(note 21 12 22 "" NOTE_INSN_BLOCK_BEG)

(note 22 21 23 "" NOTE_INSN_BLOCK_END)

(code_label 23 22 24 2 "")

(note 24 23 26 "" NOTE_INSN_DELETED)

(insn 26 24 27 (set (reg/i:DI 0 $0)
        (reg/v/u:DI 69)) -1 (nil)
    (nil))

(insn 27 26 28 (use (reg/i:DI 0 $0)) -1 (nil)
    (nil))

(jump_insn 28 27 29 (set (pc)
        (label_ref 32)) -1 (nil)
    (nil))

(barrier 29 28 30)

(note 30 29 32 "" NOTE_INSN_FUNCTION_END)

(code_label 32 30 0 1 "")

;; Function int main()

(note 2 0 3 "" NOTE_INSN_DELETED)

(note 3 2 4 "" NOTE_INSN_DELETED)

(note 4 3 5 "" NOTE_INSN_FUNCTION_BEG)

(note 5 4 7 "" NOTE_INSN_DELETED)

(note 7 5 8 "" NOTE_INSN_BLOCK_BEG)

(note 8 7 10 "" NOTE_INSN_DELETED)

(note 10 8 11 "" NOTE_INSN_BLOCK_BEG)

(insn 11 10 13 (clobber (mem/s:TI (reg:DI 66))) -1 (nil)
    (nil))

(insn 13 11 15 (set (mem/s:DI (reg:DI 66))
        (subreg:DI (reg/v:TI 69) 0)) -1 (nil)
    (nil))

(insn 15 13 17 (set (mem/s:DI (plus:DI (reg:DI 66)
                (const_int 8)))
        (subreg:DI (reg/v:TI 69) 1)) -1 (nil)
    (nil))

(insn 17 15 19 (set (reg/v:DI 70)
        (reg:DI 66)) -1 (nil)
    (nil))

(insn 19 17 21 (set (reg:DI 72)
        (symbol_ref/u:DI ("*$C32"))) -1 (nil)
    (expr_list:REG_EQUAL (symbol_ref/u:DI ("*$C32"))
        (nil)))

(insn 21 19 23 (set (reg/v:DF 71)
        (mem/u:DF (reg:DI 72))) -1 (nil)
    (nil))

(insn 23 21 24 (set (reg/v:DF 73)
        (const_double:DF (cc0) 0 0)) -1 (nil)
    (nil))

(insn/i 24 23 25 (set (mem/s:DF (reg/v:DI 70))
        (reg/v:DF 71)) -1 (nil)
    (nil))

(insn/i 25 24 26 (set (mem/s:DF (plus:DI (reg/v:DI 70)
                (const_int 8)))
        (const_double:DF (cc0) 0 0)) 243 {movsf-1} (nil)
    (nil))

(note/i 26 25 27 "" NOTE_INSN_BLOCK_BEG)

(note/i 27 26 28 "" NOTE_INSN_BLOCK_END)

(code_label/i 28 27 29 5 "")

(insn/i 29 28 30 (set (reg:DI 74)
        (reg/v:DI 70)) -1 (nil)
    (nil))

(jump_insn/i 30 29 31 (set (pc)
        (label_ref 32)) -1 (nil)
    (nil))

(barrier/i 31 30 32)

(code_label/i 32 31 33 4 "")

(note 33 32 35 "" NOTE_INSN_BLOCK_END)

(note 35 33 37 "" NOTE_INSN_DELETED)

(insn 37 35 39 (set (reg:DI 76)
        (symbol_ref/u:DI ("*$C32"))) -1 (nil)
    (expr_list:REG_EQUAL (symbol_ref/u:DI ("*$C32"))
        (nil)))

(insn 39 37 40 (set (reg:DF 75)
        (mem/u:DF (reg:DI 76))) -1 (nil)
    (nil))

(insn 40 39 41 (set (reg:DF 77)
        (eq:DF (subreg:DF (reg/v:TI 69) 0)
            (reg:DF 75))) -1 (nil)
    (nil))

(jump_insn 41 40 42 (set (pc)
        (if_then_else (eq (reg:DF 77)
                (const_double:DF (cc0) 0 0))
            (label_ref 46)
            (pc))) -1 (nil)
    (nil))

(insn 42 41 43 (set (reg:DF 78)
        (eq:DF (subreg:DF (reg/v:TI 69) 1)
            (const_double:DF (cc0) 0 0))) -1 (nil)
    (nil))

(jump_insn 43 42 44 (set (pc)
        (if_then_else (eq (reg:DF 78)
                (const_double:DF (cc0) 0 0))
            (label_ref 46)
            (pc))) -1 (nil)
    (nil))

(jump_insn 44 43 45 (set (pc)
        (label_ref 54)) -1 (nil)
    (nil))

(barrier 45 44 46)

(code_label 46 45 48 7 "")

(note 48 46 49 "" NOTE_INSN_DELETED)

(note 49 48 52 "" NOTE_INSN_DELETED)

(call_insn 52 49 53 (parallel[ 
            (call (mem:DI (symbol_ref:DI ("abort")))
                (const_int 0))
            (clobber (reg:DI 27 $27))
            (clobber (reg:DI 26 $26))
        ] ) -1 (nil)
    (nil)
    (nil))

(barrier 53 52 54)

(code_label 54 53 56 6 "")

(note 56 54 58 "" NOTE_INSN_DELETED)

(insn 58 56 59 (set (reg/i:DI 0 $0)
        (const_int 0)) -1 (nil)
    (nil))

(insn 59 58 60 (use (reg/i:DI 0 $0)) -1 (nil)
    (nil))

(jump_insn 60 59 61 (set (pc)
        (label_ref 71)) -1 (nil)
    (nil))

(barrier 61 60 62)

(note 62 61 63 "" NOTE_INSN_BLOCK_END)

(note 63 62 65 "" NOTE_INSN_DELETED)

(insn 65 63 66 (set (reg/i:DI 0 $0)
        (const_int 0)) -1 (nil)
    (nil))

(insn 66 65 67 (use (reg/i:DI 0 $0)) -1 (nil)
    (nil))

(jump_insn 67 66 68 (set (pc)
        (label_ref 71)) -1 (nil)
    (nil))

(barrier 68 67 69)

(note 69 68 71 "" NOTE_INSN_FUNCTION_END)

(code_label 71 69 0 3 "")

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

* Re: A new alpha bug
  1997-09-12 18:08 ` Richard Henderson
  1997-09-12 18:25   ` H.J. Lu
       [not found]   ` <m0x9gxl-0004ecC.cygnus.egcs@ocean.lucon.org>
@ 1997-09-13 14:44   ` H.J. Lu
  2 siblings, 0 replies; 18+ messages in thread
From: H.J. Lu @ 1997-09-13 14:44 UTC (permalink / raw)
  To: rth; +Cc: egcs

I think TImode handling is broken in egcs, maybe gcc. If I add
a dummy to complex_double to make TImode go away, the code
compiles/runs fine.

# gcc -O foo.cc
# a.out

-- 
H.J. Lu (hjl@gnu.ai.mit.edu)
---
class complex_double
{
public:
  complex_double (double r = 0, double i = 0): re (r), im (i) { }
  double re, im;
  int dummy;
};

main()
{
  complex_double one = 1.0;
  // complex_double one (1.0);
  if (one.re != 1.0 || one.im != 0.0)
    abort ();

  return 0;
}

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

* Re: A new alpha bug
  1997-09-12 19:10     ` Jason Merrill
  1997-09-12 22:03       ` H.J. Lu
@ 1997-09-14 11:40       ` H.J. Lu
  1997-09-14 11:56       ` More info on the g++ " H.J. Lu
  2 siblings, 0 replies; 18+ messages in thread
From: H.J. Lu @ 1997-09-14 11:40 UTC (permalink / raw)
  To: Jason Merrill; +Cc: egcs, rth

> > complex_double one = 1.0;
> 
> complex_double tmp (1.0, 0);
> complex_double one (tmp);
> 

Well, it looks like a g++ and alpha/TImode bug. The default copy
constructor on alpha generated by g++ is bogus when TImode is used.
If a copy constructor is defined, the code compils/runs fine.

# gcc -O foo.cc
# a.out


-- 
H.J. Lu (hjl@gnu.ai.mit.edu)
----
class complex_double
{
public:
  complex_double (double r = 0, double i = 0): re (r), im (i) { }
  complex_double (const complex_double &x)
  {
    re = x.re;
    im = x.im;
  }

  double re, im;
};

main()
{
  complex_double one = 1.0;
  if (one.re != 1.0 || one.im != 0.0)
    abort ();

  return 0;
}

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

* More info on the g++ alpha bug.
  1997-09-12 19:10     ` Jason Merrill
  1997-09-12 22:03       ` H.J. Lu
  1997-09-14 11:40       ` H.J. Lu
@ 1997-09-14 11:56       ` H.J. Lu
  2 siblings, 0 replies; 18+ messages in thread
From: H.J. Lu @ 1997-09-14 11:56 UTC (permalink / raw)
  To: Jason Merrill; +Cc: rth, egcs

Hi,

It seems that -fansi-overloading is broken on alpha.

# gcc -O foo.cc
# a.out
Floating point exception (core dumped)
# gcc -O foo.cc -fno-ansi-overloading
# a.out



H.J.
---foo.cc-
class complex_double
{
public:
  complex_double (double r = 0, double i = 0): re (r), im (i) { }
  double re, im;
};

main()
{
  complex_double one = 1.0;
  if (one.re != 1.0 || one.im != 0.0)
    abort ();

  return 0;
}

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

* Re: A new alpha bug
  1997-09-12 15:31 A new alpha bug H.J. Lu
  1997-09-12 18:08 ` Richard Henderson
@ 1997-09-15 20:26 ` Jim Wilson
  1997-09-15 20:38   ` Jason Merrill
  1 sibling, 1 reply; 18+ messages in thread
From: Jim Wilson @ 1997-09-15 20:26 UTC (permalink / raw)
  To: H.J. Lu; +Cc: egcs, g++

It is a problem with TARGET_EXPR handling.

The C++ front end creates a TARGET_EXPR with an RTL-less VAR_DECL in slot.
Later, there is some kind of argument conversion (for the new call?) that
adds an ADDR_EXPR and sets TREE_ADRESSABLE for the VAR_DECL.  When we call
expand_expr, a TImode pseudo is passed in for target.  The TARGET_EXPR
code sees that there is no RTL, and that we have a target, so it stores
the pseudo into the VAR_DECL.  We now have an addressable VAR_DECL with a
register in DECL_RTL.  This is very bad.

mark_addressable is eventually called, but since TREE_ADDRESSABLE is already
set, nothing happens.  When we evaluate the ADDR_EXPR, it notices that we
have a REG, so it creates a stack slot, stores the REG into it, and returns
the stack slot, causing the result of the constructor to be stored into the
stack slot.  The TARGET_EXPR however still has the REG in DECL_RTL (slot),
so we return the REG, and subsequent codes uses the REG which has never been
stored into.

The following patch will fix the problem, but I am not sure if it is the
right solution.

Mon Sep 15 20:13:05 1997  Jim Wilson  <wilson@cygnus.com>

	* expr.c (expand_expr, case TARGET_EXPR): Don't use target if
	TREE_ADDRESSABLE (slot) is true.

Index: expr.c
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/expr.c,v
retrieving revision 1.3
diff -p -r1.3 expr.c
*** expr.c	1997/09/03 05:33:06	1.3
--- expr.c	1997/09/16 03:12:34
*************** expand_expr (exp, target, tmode, modifie
*** 6902,6908 ****
  	if (! ignore)
  	  target = original_target;
  
! 	if (target == 0)
  	  {
  	    if (DECL_RTL (slot) != 0)
  	      {
--- 6902,6911 ----
  	if (! ignore)
  	  target = original_target;
  
! 	/* We can't use the pseudo passed in as target if SLOT must be
! 	   addressable.  */
! 
! 	if (target == 0 || TREE_ADDRESSABLE (slot))
  	  {
  	    if (DECL_RTL (slot) != 0)
  	      {


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

* Re: A new alpha bug
  1997-09-15 20:26 ` Jim Wilson
@ 1997-09-15 20:38   ` Jason Merrill
  1997-09-16 10:11     ` H.J. Lu
  1997-09-16 11:39     ` Jim Wilson
  0 siblings, 2 replies; 18+ messages in thread
From: Jason Merrill @ 1997-09-15 20:38 UTC (permalink / raw)
  To: Jim Wilson; +Cc: H.J. Lu, egcs, g++

>>>>> Jim Wilson <wilson@cygnus.com> writes:

> The following patch will fix the problem, but I am not sure if it is the
> right solution.

I think this is better:

Wed Aug 13 17:32:38 1997  Jason Merrill  <jason@yorick.cygnus.com>

	* expr.c (expand_expr, case TARGET_EXPR): Call mark_addressable
 	again for the slot after we give it RTL.

Index: expr.c
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/expr.c,v
retrieving revision 1.7
diff -c -r1.7 expr.c
*** expr.c	1997/09/16 02:07:20	1.7
--- expr.c	1997/09/16 03:37:26
***************
*** 6919,6924 ****
--- 6919,6929 ----
  		/* All temp slots at this level must not conflict.  */
  		preserve_temp_slots (target);
  		DECL_RTL (slot) = target;
+ 		if (TREE_ADDRESSABLE (slot))
+ 		  {
+ 		    TREE_ADDRESSABLE (slot) = 0;
+ 		    mark_addressable (slot);
+ 		  }
  
  		/* Since SLOT is not known to the called function
  		   to belong to its stack frame, we must build an explicit

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

* Re: A new alpha bug
  1997-09-15 20:38   ` Jason Merrill
@ 1997-09-16 10:11     ` H.J. Lu
  1997-09-16 10:18       ` Jason Merrill
  1997-09-16 11:39       ` Jim Wilson
  1997-09-16 11:39     ` Jim Wilson
  1 sibling, 2 replies; 18+ messages in thread
From: H.J. Lu @ 1997-09-16 10:11 UTC (permalink / raw)
  To: Jason Merrill; +Cc: wilson, egcs, g++

> 
> >>>>> Jim Wilson <wilson@cygnus.com> writes:
> 
> > The following patch will fix the problem, but I am not sure if it is the
> > right solution.
> 
> I think this is better:
> 
> Wed Aug 13 17:32:38 1997  Jason Merrill  <jason@yorick.cygnus.com>
> 
> 	* expr.c (expand_expr, case TARGET_EXPR): Call mark_addressable
>  	again for the slot after we give it RTL.
> 
> Index: expr.c
> ===================================================================
> RCS file: /cvs/cvsfiles/egcs/gcc/expr.c,v
> retrieving revision 1.7
> diff -c -r1.7 expr.c
> *** expr.c	1997/09/16 02:07:20	1.7
> --- expr.c	1997/09/16 03:37:26
> ***************
> *** 6919,6924 ****
> --- 6919,6929 ----
>   		/* All temp slots at this level must not conflict.  */
>   		preserve_temp_slots (target);
>   		DECL_RTL (slot) = target;
> + 		if (TREE_ADDRESSABLE (slot))
> + 		  {
> + 		    TREE_ADDRESSABLE (slot) = 0;
> + 		    mark_addressable (slot);
> + 		  }
>   
>   		/* Since SLOT is not known to the called function
>   		   to belong to its stack frame, we must build an explicit
> 

Have you tried your patch? on a cross compiler? I need
this patch to get it to generate the right asm code with my cross
compiler. It takes hours to verify on my alpha machine.

BTW, both yours and Jim's patches sound like a kludge to me.
I don't understand why it takes TImode to expose this bug.
Is there a more subtle bug somehwhere else? Also how many
other places do we have to change to make it work right
for all cases? Is there a cleaner way to fix it?

Thanks.

-- 
H.J. Lu (hjl@gnu.ai.mit.edu)
----
Index: expr.c
===================================================================
RCS file: /home/work/cvs/gnu/egcs/gcc/expr.c,v
retrieving revision 1.1.1.4
diff -u -r1.1.1.4 expr.c
--- expr.c	1997/09/11 20:47:02	1.1.1.4
+++ expr.c	1997/09/16 16:59:11
@@ -6918,6 +6920,11 @@
 		/* All temp slots at this level must not conflict.  */
 		preserve_temp_slots (target);
 		DECL_RTL (slot) = target;
+		if (TREE_ADDRESSABLE (slot))
+		  {
+		    TREE_ADDRESSABLE (slot) = 0;
+		    mark_addressable (slot);
+		  }
 
 		/* Since SLOT is not known to the called function
 		   to belong to its stack frame, we must build an explicit
@@ -6951,6 +6958,11 @@
 	      }
 
 	    DECL_RTL (slot) = target;
+	    if (TREE_ADDRESSABLE (slot))
+	      {
+		TREE_ADDRESSABLE (slot) = 0;
+		mark_addressable (slot);
+	      }
 	  }
 
 	exp1 = TREE_OPERAND (exp, 3) = TREE_OPERAND (exp, 1);

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

* Re: A new alpha bug
  1997-09-16 10:11     ` H.J. Lu
@ 1997-09-16 10:18       ` Jason Merrill
  1997-09-16 11:09         ` H.J. Lu
  1997-09-16 11:39       ` Jim Wilson
  1 sibling, 1 reply; 18+ messages in thread
From: Jason Merrill @ 1997-09-16 10:18 UTC (permalink / raw)
  To: H.J. Lu; +Cc: wilson, egcs, g++

>>>>> H J Lu <hjl@lucon.org> writes:

> Have you tried your patch? on a cross compiler?

Not to the alpha, no.  Sorry.

> I need this patch to get it to generate the right asm code with my cross
> compiler.

Yep, looks right.

> BTW, both yours and Jim's patches sound like a kludge to me.
> I don't understand why it takes TImode to expose this bug.
> Is there a more subtle bug somehwhere else? Also how many
> other places do we have to change to make it work right
> for all cases? Is there a cleaner way to fix it?

I don't imagine anyplace else needs to be changed.  Bashing the RTL for a
variable is not a common operation.

Jason

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

* Re: A new alpha bug
  1997-09-16 11:09         ` H.J. Lu
@ 1997-09-16 10:38           ` Jason Merrill
  0 siblings, 0 replies; 18+ messages in thread
From: Jason Merrill @ 1997-09-16 10:38 UTC (permalink / raw)
  To: H.J. Lu; +Cc: wilson, egcs, g++

>>>>> H J Lu <hjl@lucon.org> writes:

> I have seen places with

> 	TREE_ADDRESSABLE (init) = 1;
> 	DECL_RTL (init) = return_target;

> or something like it. Should they be replaces by

> 	DECL_RTL (init) = return_target;
> 	TREE_ADDRESSABLE (init) = 0;
> 	mark_addressable (init);

If you're referring to the code in cplus_expand_expr, no.  That decl is
a wrapper around specific RTL, not a real variable.

Jason

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

* Re: A new alpha bug
  1997-09-16 10:18       ` Jason Merrill
@ 1997-09-16 11:09         ` H.J. Lu
  1997-09-16 10:38           ` Jason Merrill
  0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 1997-09-16 11:09 UTC (permalink / raw)
  To: Jason Merrill; +Cc: wilson, egcs, g++

> > BTW, both yours and Jim's patches sound like a kludge to me.
> > I don't understand why it takes TImode to expose this bug.
> > Is there a more subtle bug somehwhere else? Also how many
> > other places do we have to change to make it work right
> > for all cases? Is there a cleaner way to fix it?
> 
> I don't imagine anyplace else needs to be changed.  Bashing the RTL for a
> variable is not a common operation.
> 

I have seen places with

	TREE_ADDRESSABLE (init) = 1;
	DECL_RTL (init) = return_target;

or something like it. Should they be replaces by


	DECL_RTL (init) = return_target;
	TREE_ADDRESSABLE (init) = 0;
	mark_addressable (init);

Maybe we should make a function/macro out of it and use it:

#define MARK_TREE_ADDRESSABLE(node) \
	if (TREE_ADDRESSABLE ((node)) \
	  { \
	    TREE_ADDRESSABLE ((node)) = 0;
	    mark_addressable ((node));
	  }

	TREE_ADDRESSABLE (init) = 1;
	DECL_RTL (init) = return_target;
	MARK_TREE_ADDRESSABLE (init);

	
-- 
H.J. Lu (hjl@gnu.ai.mit.edu)

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

* Re: A new alpha bug
  1997-09-16 10:11     ` H.J. Lu
  1997-09-16 10:18       ` Jason Merrill
@ 1997-09-16 11:39       ` Jim Wilson
  1 sibling, 0 replies; 18+ messages in thread
From: Jim Wilson @ 1997-09-16 11:39 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Jason Merrill, egcs, g++

	BTW, both yours and Jim's patches sound like a kludge to me.

I don't see anything kludgey about it.  The patch is just making sure that
we create proper RTL for a VAR_DECL.

	I don't understand why it takes TImode to expose this bug.

These kinds of things happen all of the time.  Gcc is so complicated that
often it isn't clear why a particular set of circumstances is necessary to
trigger a bug.

In this case, if the mode is larger than TImode, then the structure won't be
allocated to a pseudo, and the problem won't arise.  This is why the problem
goes away when you add a dummy variable.

I haven't tried to figure out if the problem can be reproduced with smaller
modes.

	Is there a more subtle bug somehwhere else?

I don't think so.

	Also how many
	other places do we have to change to make it work right
	for all cases?

There is only one place that needs to be fixed.  You were confused into
thinking that more than one place was broken because of an accidental mistake
in Jason's patch.

	Is there a cleaner way to fix it?

Not that I am aware of.

Jim

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

* Re: A new alpha bug
  1997-09-15 20:38   ` Jason Merrill
  1997-09-16 10:11     ` H.J. Lu
@ 1997-09-16 11:39     ` Jim Wilson
  1 sibling, 0 replies; 18+ messages in thread
From: Jim Wilson @ 1997-09-16 11:39 UTC (permalink / raw)
  To: Jason Merrill; +Cc: H.J. Lu, egcs, g++

	I think this is better:

	Wed Aug 13 17:32:38 1997  Jason Merrill  <jason@yorick.cygnus.com>

	* expr.c (expand_expr, case TARGET_EXPR): Call mark_addressable
 	again for the slot after we give it RTL.

Actually, this patch is wrong, because you accidentally inserted your hunk of
code at the wrong place.  This is why the patch did not work for H.J. Lu.

The idea seems OK though.  However, if we are going to do this, then I think we
should add an `else' clause, so as to avoid doing this work when it is not
necessary.

I will add the following patch to the egcs sources after I have verified that
it works.

Tue Sep 16 11:13:46 1997  Jim Wilson  <wilson@cygnus.com>

	* expr.c (expand_expr): Remove previous incorrect change.
	If target and slot has no DECL_RTL, then call mark_addressable
	again for the slot after we give it RTL.

Index: expr.c
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/expr.c,v
retrieving revision 1.8
diff -p -r1.8 expr.c
*** expr.c	1997/09/16 07:53:51	1.8
--- expr.c	1997/09/16 18:13:23
*************** expand_expr (exp, target, tmode, modifie
*** 6919,6929 ****
  		/* All temp slots at this level must not conflict.  */
  		preserve_temp_slots (target);
  		DECL_RTL (slot) = target;
- 		if (TREE_ADDRESSABLE (slot))
- 		  {
- 		    TREE_ADDRESSABLE (slot) = 0;
- 		    mark_addressable (slot);
- 		  }
  
  		/* Since SLOT is not known to the called function
  		   to belong to its stack frame, we must build an explicit
--- 6919,6924 ----
*************** expand_expr (exp, target, tmode, modifie
*** 6955,6962 ****
                  if (TREE_OPERAND (exp, 1) == NULL_TREE)
                    return target;
  	      }
! 
! 	    DECL_RTL (slot) = target;
  	  }
  
  	exp1 = TREE_OPERAND (exp, 3) = TREE_OPERAND (exp, 1);
--- 6950,6966 ----
                  if (TREE_OPERAND (exp, 1) == NULL_TREE)
                    return target;
  	      }
! 	    else
! 	      {
! 		DECL_RTL (slot) = target;
! 		/* If we must have an addressable slot, then make sure that
! 		   the RTL that we just stored in slot is OK.  */
! 		if (TREE_ADDRESSABLE (slot))
! 		  {
! 		    TREE_ADDRESSABLE (slot) = 0;
! 		    mark_addressable (slot);
! 		  }
! 	      }
  	  }
  
  	exp1 = TREE_OPERAND (exp, 3) = TREE_OPERAND (exp, 1);

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

* Re: A new alpha bug
@ 1997-09-16 14:53 Mike Stump
  0 siblings, 0 replies; 18+ messages in thread
From: Mike Stump @ 1997-09-16 14:53 UTC (permalink / raw)
  To: hjl, wilson; +Cc: egcs, g++

This patch should be safe, and goes this the general design of
TARGET_EXPR and the C++ frontend's use of it, though, it could be made
better by checking what target is...  Only if it is (or might be) a
REG, do we have to worry about doing this.  If it is a mem, we should
be safe.

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

* Re:  A new alpha bug
@ 1997-09-12 19:59 Kaveh R. Ghazi
  0 siblings, 0 replies; 18+ messages in thread
From: Kaveh R. Ghazi @ 1997-09-12 19:59 UTC (permalink / raw)
  To: egcs, hjl; +Cc: rth

 > From: hjl@lucon.org (H.J. Lu)
 > Subject: A new alpha bug
 > 
 > Using egcs 970910 configured for linux/alpha, this code failed
 > to run on alpha with any optimization:
 > 
 > # gcc foo.cc
 > # a.out
 > # gcc -O foo.cc
 > # a.out
 > Floating point exception (core dumped)
 > 
 > gcc 2.7.2.1 for linux/alpha is fine.
 > -- 
 > H.J. Lu (hjl@gnu.ai.mit.edu)
 > ---
 > class complex_double
 > {
 > public:
 >   complex_double (double r = 0, double i = 0): re (r), im (i) { }
 >   double re, im;
 > };
 > 
 > main()
 > {
 >   complex_double one = 1.0;
 >   if (one.re != 1.0 || one.im != 0.0)
 >     abort ();
 > 
 >   return 0;
 > }

	FWIW, egcs-2.90.07 970910 had the same failure on alpha-dec-osf4.0.

		--Kaveh
--
Kaveh R. Ghazi				Project Manager
ghazi@caip.rutgers.edu			ICon CMT Corp.

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

end of thread, other threads:[~1997-09-16 14:53 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1997-09-12 15:31 A new alpha bug H.J. Lu
1997-09-12 18:08 ` Richard Henderson
1997-09-12 18:25   ` H.J. Lu
     [not found]   ` <m0x9gxl-0004ecC.cygnus.egcs@ocean.lucon.org>
1997-09-12 19:10     ` Jason Merrill
1997-09-12 22:03       ` H.J. Lu
1997-09-14 11:40       ` H.J. Lu
1997-09-14 11:56       ` More info on the g++ " H.J. Lu
1997-09-13 14:44   ` A new " H.J. Lu
1997-09-15 20:26 ` Jim Wilson
1997-09-15 20:38   ` Jason Merrill
1997-09-16 10:11     ` H.J. Lu
1997-09-16 10:18       ` Jason Merrill
1997-09-16 11:09         ` H.J. Lu
1997-09-16 10:38           ` Jason Merrill
1997-09-16 11:39       ` Jim Wilson
1997-09-16 11:39     ` Jim Wilson
1997-09-12 19:59 Kaveh R. Ghazi
1997-09-16 14:53 Mike Stump

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