Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-18 Thread Steve Ellcey 
On Sat, 2011-04-16 at 00:49 +0200, Michael Matz wrote:

 Callers of expand_expr are expected to deal with the situation that the 
 returned object doesn't have the desired mode, hence I think it's okay for 
 expand_expr to return a DImode code_label rtx.  Meaning we have to deal 
 with it.  The patch of HJ is a first step but doesn't cater for op0 being 
 a CONST_INT, which doesn't have a mode, hence at the very least the code 
 should look like:
 
 enum machine_mode inner_mode = GET_MODE (op0);
 if (inner_mode == VOIDmode)
   inner_mode = TYPE_MODE (inner_type);
 
 I do wonder what other places in expand really would need something 
 similar.  Right now nothing else ICEs but perhaps silently generates code 
 that only works by accident.  Well, I guess we'll see.
 
 
 Ciao,
 Michael.

I tested your patch and it fixed the problem with labels-3.c.  It also
ran through the test suite with no regressions on IA64 HP-UX and Linux
and on x86 Linux.  I do see other places in the compiler where we call
GET_MODE and then check for VOIDmode to do something special/different
if we got that return value (in cfgexpand.c for example) so maybe this
is just one more place where we need this type of check.

Steve Ellcey
s...@cup.hp.com

Can someone approve this patch?



2011-05-18  Michael Matz  m...@suse.de
Steve Ellcey  s...@cup.hp.com

* expr.c (expand_expr_real_2): Use GET_MODE instead of TYPE_MODE.


Index: expr.c
===
--- expr.c  (revision 172632)
+++ expr.c  (working copy)
@@ -7360,8 +7360,11 @@
   else if (CONSTANT_P (op0))
{
  tree inner_type = TREE_TYPE (treeop0);
- enum machine_mode inner_mode = TYPE_MODE (inner_type);
+ enum machine_mode inner_mode = GET_MODE (op0);
 
+ if (inner_mode == VOIDmode)
+   inner_mode = TYPE_MODE (inner_type);
+
  if (modifier == EXPAND_INITIALIZER)
op0 = simplify_gen_subreg (mode, op0, inner_mode,
   subreg_lowpart_offset (mode,

Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-18 Thread Richard Guenther 
On Mon, Apr 18, 2011 at 7:52 PM, Steve Ellcey s...@cup.hp.com wrote:
 On Sat, 2011-04-16 at 00:49 +0200, Michael Matz wrote:

 Callers of expand_expr are expected to deal with the situation that the
 returned object doesn't have the desired mode, hence I think it's okay for
 expand_expr to return a DImode code_label rtx.  Meaning we have to deal
 with it.  The patch of HJ is a first step but doesn't cater for op0 being
 a CONST_INT, which doesn't have a mode, hence at the very least the code
 should look like:

     enum machine_mode inner_mode = GET_MODE (op0);
     if (inner_mode == VOIDmode)
       inner_mode = TYPE_MODE (inner_type);

 I do wonder what other places in expand really would need something
 similar.  Right now nothing else ICEs but perhaps silently generates code
 that only works by accident.  Well, I guess we'll see.


 Ciao,
 Michael.

 I tested your patch and it fixed the problem with labels-3.c.  It also
 ran through the test suite with no regressions on IA64 HP-UX and Linux
 and on x86 Linux.  I do see other places in the compiler where we call
 GET_MODE and then check for VOIDmode to do something special/different
 if we got that return value (in cfgexpand.c for example) so maybe this
 is just one more place where we need this type of check.

 Steve Ellcey
 s...@cup.hp.com

 Can someone approve this patch?

Ok.

Thanks,
Richard.



 2011-05-18  Michael Matz  m...@suse.de
            Steve Ellcey  s...@cup.hp.com

        * expr.c (expand_expr_real_2): Use GET_MODE instead of TYPE_MODE.


 Index: expr.c
 ===
 --- expr.c      (revision 172632)
 +++ expr.c      (working copy)
 @@ -7360,8 +7360,11 @@
       else if (CONSTANT_P (op0))
        {
          tree inner_type = TREE_TYPE (treeop0);
 -         enum machine_mode inner_mode = TYPE_MODE (inner_type);
 +         enum machine_mode inner_mode = GET_MODE (op0);

 +         if (inner_mode == VOIDmode)
 +           inner_mode = TYPE_MODE (inner_type);
 +
          if (modifier == EXPAND_INITIALIZER)
            op0 = simplify_gen_subreg (mode, op0, inner_mode,
                                       subreg_lowpart_offset (mode,

Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-15 Thread Steve Ellcey 
I was curious if anyone was still looking at this problem?  I see this
on IA64 HP-UX in 32 bit mode where ptr_mode(SImode) != Pmode(DImode).
As H.J. points out, expand_expr_real_2 is calling simplify_gen_subreg
(expr.c, line 7366) and at that point op0 is (label_ref/v:DI 32) and
innermode is SImode.  Because innermode doesn't match the mode of op0,
we abort in simplify_subreg.  So it seems we either need to change how
we calculate innermode (so that it is DImode) or change expand_expr so
that it returns a SImode RTX expression for the tree treeop0.  I am not
sure which of these is the right thing to do.  The tree (treeop0) that
op0 is generated from is below and when we call expr_expr the modifier
is EXPAND_INITIALIZER.  Should expand_expr return an SImode expression
for this tree (ptr_mode) or a DImode expression (Pmode) in this case?

 addr_expr 65866560
type pointer_type 657d8960
type void_type 657d8900 void VOID
align 8 symtab 0 alias set -1 canonical type 657d8900
pointer_to_this pointer_type 657d8960
sizes-gimplified public unsigned SI
size integer_cst 657d1920 constant 32
unit size integer_cst 657d16c0 constant 4
align 32 symtab 0 alias set -1 canonical type 657d8960
pointer_to_this pointer_type 657f1720 reference_to_this 
reference_type 657f14e0
constant
arg 0 label_decl 657e21b8 l2 type void_type 657d8900 void
side-effects VOID file labels-3.c line 16 col 2
align 1 context function_decl 65858180 foo initial error_mark 
657e6040
(code_label/s 32 0 0 4 4 (l2) [2 uses])

chain var_decl 6585e3c0 p type pointer_type 657d8960
used unsigned SI file labels-3.c line 12 col 9 size integer_cst 
657d1920 32 unit size integer_cst 657d16c0 4
align 32 context function_decl 65858180 foo
(mem/f/c/i:SI (reg/f:DI 328 sfp) [0 p+0 S4 A32])
labels-3.c:11:44



Steve Ellcey
s...@cup.hp.com

Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-07 Thread Richard Guenther 
On Wed, Apr 6, 2011 at 7:03 PM, H.J. Lu hjl.to...@gmail.com wrote:
 On Tue, Apr 5, 2011 at 3:29 AM, Richard Guenther
 richard.guent...@gmail.com wrote:
 On Tue, Apr 5, 2011 at 8:44 AM, Paolo Bonzini bonz...@gnu.org wrote:
 Index: cgraphbuild.c
 ===
 --- cgraphbuild.c.orig  2011-04-03 11:28:45.0 +0200
 +++ cgraphbuild.c       2011-04-03 11:31:21.0 +0200
 @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
   tree decl;
   struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;

 +  t = canonicalize_constructor_val (t);
 +  if (!t)
 +    t = *tp;
 +  else if (t != *tp)
 +    *tp = t;
 +
   switch (TREE_CODE (t))
     {
     case VAR_DECL:

 This change caused:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440


 This avoids  canonicalizing constructor values for address conversion
 if Pmode != ptr_mode.  OK for trunk?

 Certainly the right place to fix it is in canonicalize_constructor_val 
 itself.

 There should never be any Pmode pointer types in the gimple IL.  The
 proper place to fix it if any is in useless_type_conversion_p.


 We have

 5600      newx = simplify_subreg (outermode, op, innermode, byte);
 (gdb) f 1
 #1  0x00708494 in expand_expr_real_2 (ops=0x7fffb0c0, target=0x0,
    tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
    at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
 7366                op0 = simplify_gen_subreg (mode, op0, inner_mode,
 (gdb) call debug_tree (treeop0)
  addr_expr 0x70a78d50
    type pointer_type 0x70b83f18
        type void_type 0x70b83e70 void VOID
            align 8 symtab 0 alias set -1 canonical type 0x70b83e70
            pointer_to_this pointer_type 0x70b83f18
        sizes-gimplified public unsigned SI
        size integer_cst 0x70b706e0 constant 32
        unit size integer_cst 0x70b703e8 constant 4
        align 32 symtab 0 alias set -1 canonical type 0x70b83f18
        pointer_to_this pointer_type 0x70b985e8
    constant
    arg 0 label_decl 0x70b7b400 l2 type void_type 0x70b83e70 void
        side-effects VOID file x.i line 8 col 2
        align 1 context function_decl 0x70a68f00 foo initial
 error_mark 0x70b789f0
        (code_label/s 22 0 0 4 4 (l2) [2 uses])

        chain var_decl 0x70a790a0 p type pointer_type 0x70b83f18
            used unsigned SI file x.i line 4 col 9 size integer_cst
 0x70b706e0 32 unit size integer_cst 0x70b703e8 4
            align 32 context function_decl 0x70a68f00 foo
            (mem/f/c/i:SI (plus:DI (reg/f:DI 20 frame)
        (const_int -4 [0xfffc])) [0 p+0 S4 A32])
    x.i:3:44
 (gdb) call debug_rtx (op0)
 (label_ref/v:DI 22)
 (gdb)

 Since ptr_mode != Pmode, the type of op0 != type of treeop0.
 Should we use GET_MODE (op0) instead of TYPE_MODE (inner_type)
 here? Does this patch make any senses?

First I wonder what CONSTANT_P object we arrive with here (it looks like
something unfolded, given that we likely came here with a NOP_EXPR).

Second, no, it doesn't make sense as CONST_INTs are modeless
(which is exactly why we look at tree types here ...)

The above gdb session paste is from a totally different place, so I can't
match the data to the place you are trying to patch.

Richard.


 --
 H.J.
 ---
 diff --git a/gcc/expr.c b/gcc/expr.c
 index d521f64..439f245 100644
 --- a/gcc/expr.c
 +++ b/gcc/expr.c
 @@ -7360,7 +7360,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum 
 machine_m
 ode tmode,
       else if (CONSTANT_P (op0))
        {
          tree inner_type = TREE_TYPE (treeop0);
 -         enum machine_mode inner_mode = TYPE_MODE (inner_type);
 +         enum machine_mode inner_mode = GET_MODE (op0);

          if (modifier == EXPAND_INITIALIZER)
            op0 = simplify_gen_subreg (mode, op0, inner_mode,

Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-07 Thread Michael Matz 
Hi,

On Thu, 7 Apr 2011, Richard Guenther wrote:

  5600      newx = simplify_subreg (outermode, op, innermode, byte);
  (gdb) f 1
  #1  0x00708494 in expand_expr_real_2 (ops=0x7fffb0c0, 
  target=0x0,
     tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
     at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
  7366                op0 = simplify_gen_subreg (mode, op0, inner_mode,
  (gdb) call debug_tree (treeop0)
   addr_expr 0x70a78d50
     type pointer_type 0x70b83f18
         type void_type 0x70b83e70 void VOID
             align 8 symtab 0 alias set -1 canonical type 0x70b83e70
             pointer_to_this pointer_type 0x70b83f18
         sizes-gimplified public unsigned SI
     arg 0 label_decl 0x70b7b400 l2 type void_type 0x70b83e70 void
  (gdb) call debug_rtx (op0)
  (label_ref/v:DI 22)
  (gdb)
 
 
 First I wonder what CONSTANT_P object we arrive with here (it looks like
 something unfolded, given that we likely came here with a NOP_EXPR).

The CONSTANT_P object is the '(label_ref/v:DI 22)'.  Not only CONST_INT 
are CONSTANT_P, also some others (symbol_ref too).  Those might have a 
mode.  Here the label_ref has DImode, but the pointer type in trees points 
to an SImode.  The latter makes sense, because that's what ptr_mode is for 
HJs target.

HJ: you'll need to tell us what you mean with 'breaks 
gcc.c-torture/compile/labels-3.c' .  What breaks, in which way?


Ciao,
Michael.

Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-07 Thread H.J. Lu 
On Thu, Apr 7, 2011 at 5:34 AM, Michael Matz m...@suse.de wrote:
 Hi,

 On Thu, 7 Apr 2011, Richard Guenther wrote:

  5600      newx = simplify_subreg (outermode, op, innermode, byte);
  (gdb) f 1
  #1  0x00708494 in expand_expr_real_2 (ops=0x7fffb0c0, 
  target=0x0,
     tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
     at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
  7366                op0 = simplify_gen_subreg (mode, op0, inner_mode,
  (gdb) call debug_tree (treeop0)
   addr_expr 0x70a78d50
     type pointer_type 0x70b83f18
         type void_type 0x70b83e70 void VOID
             align 8 symtab 0 alias set -1 canonical type 0x70b83e70
             pointer_to_this pointer_type 0x70b83f18
         sizes-gimplified public unsigned SI
     arg 0 label_decl 0x70b7b400 l2 type void_type 0x70b83e70 void
  (gdb) call debug_rtx (op0)
  (label_ref/v:DI 22)
  (gdb)
 

 First I wonder what CONSTANT_P object we arrive with here (it looks like
 something unfolded, given that we likely came here with a NOP_EXPR).

 The CONSTANT_P object is the '(label_ref/v:DI 22)'.  Not only CONST_INT
 are CONSTANT_P, also some others (symbol_ref too).  Those might have a
 mode.  Here the label_ref has DImode, but the pointer type in trees points
 to an SImode.  The latter makes sense, because that's what ptr_mode is for
 HJs target.

 HJ: you'll need to tell us what you mean with 'breaks
 gcc.c-torture/compile/labels-3.c' .  What breaks, in which way?


I got

#0  fancy_abort (
file=0x12470e0 /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c,
line=5266, function=0x1248470 simplify_subreg)
at /export/gnu/import/git/gcc-x32/gcc/diagnostic.c:893
#1  0x009d0ab5 in simplify_subreg (outermode=HImode, op=0x70c55dd0,
innermode=SImode, byte=0)
at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:5265
#2  0x009d1e83 in simplify_gen_subreg (outermode=HImode,
op=0x70c55dd0, innermode=SImode, byte=0)
at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:5600
#3  0x00708736 in expand_expr_real_2 (ops=0x7fffb480, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
#4  0x007153e1 in expand_expr_real_1 (exp=0x70a87f30, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER, alt_rtl=0x0)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:9728
..
(gdb) f 3
#3  0x00708736 in expand_expr_real_2 (ops=0x7fffb480, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
7366op0 = simplify_gen_subreg (mode, op0, inner_mode,
(gdb) call debug_rtx (op0)
(label_ref/v:DI 22)
(gdb) p  inner_mode
$2 = SImode
(gdb)

We are calling simplify_gen_subreg with wrong inner_mode.

-- 
H.J.

Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-06 Thread H.J. Lu 
On Tue, Apr 5, 2011 at 3:29 AM, Richard Guenther
richard.guent...@gmail.com wrote:
 On Tue, Apr 5, 2011 at 8:44 AM, Paolo Bonzini bonz...@gnu.org wrote:
 Index: cgraphbuild.c
 ===
 --- cgraphbuild.c.orig  2011-04-03 11:28:45.0 +0200
 +++ cgraphbuild.c       2011-04-03 11:31:21.0 +0200
 @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
   tree decl;
   struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;

 +  t = canonicalize_constructor_val (t);
 +  if (!t)
 +    t = *tp;
 +  else if (t != *tp)
 +    *tp = t;
 +
   switch (TREE_CODE (t))
     {
     case VAR_DECL:

 This change caused:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440


 This avoids  canonicalizing constructor values for address conversion
 if Pmode != ptr_mode.  OK for trunk?

 Certainly the right place to fix it is in canonicalize_constructor_val 
 itself.

 There should never be any Pmode pointer types in the gimple IL.  The
 proper place to fix it if any is in useless_type_conversion_p.


We have

5600  newx = simplify_subreg (outermode, op, innermode, byte);
(gdb) f 1
#1  0x00708494 in expand_expr_real_2 (ops=0x7fffb0c0, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
7366op0 = simplify_gen_subreg (mode, op0, inner_mode,
(gdb) call debug_tree (treeop0)
 addr_expr 0x70a78d50
type pointer_type 0x70b83f18
type void_type 0x70b83e70 void VOID
align 8 symtab 0 alias set -1 canonical type 0x70b83e70
pointer_to_this pointer_type 0x70b83f18
sizes-gimplified public unsigned SI
size integer_cst 0x70b706e0 constant 32
unit size integer_cst 0x70b703e8 constant 4
align 32 symtab 0 alias set -1 canonical type 0x70b83f18
pointer_to_this pointer_type 0x70b985e8
constant
arg 0 label_decl 0x70b7b400 l2 type void_type 0x70b83e70 void
side-effects VOID file x.i line 8 col 2
align 1 context function_decl 0x70a68f00 foo initial
error_mark 0x70b789f0
(code_label/s 22 0 0 4 4 (l2) [2 uses])

chain var_decl 0x70a790a0 p type pointer_type 0x70b83f18
used unsigned SI file x.i line 4 col 9 size integer_cst
0x70b706e0 32 unit size integer_cst 0x70b703e8 4
align 32 context function_decl 0x70a68f00 foo
(mem/f/c/i:SI (plus:DI (reg/f:DI 20 frame)
(const_int -4 [0xfffc])) [0 p+0 S4 A32])
x.i:3:44
(gdb) call debug_rtx (op0)
(label_ref/v:DI 22)
(gdb)

Since ptr_mode != Pmode, the type of op0 != type of treeop0.
Should we use GET_MODE (op0) instead of TYPE_MODE (inner_type)
here? Does this patch make any senses?


-- 
H.J.
---
diff --git a/gcc/expr.c b/gcc/expr.c
index d521f64..439f245 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -7360,7 +7360,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_m
ode tmode,
   else if (CONSTANT_P (op0))
{
  tree inner_type = TREE_TYPE (treeop0);
- enum machine_mode inner_mode = TYPE_MODE (inner_type);
+ enum machine_mode inner_mode = GET_MODE (op0);

  if (modifier == EXPAND_INITIALIZER)
op0 = simplify_gen_subreg (mode, op0, inner_mode,

PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

2011-04-04 Thread H.J. Lu 
On Mon, Apr 4, 2011 at 3:00 PM, H.J. Lu hjl.to...@gmail.com wrote:
 On Sun, Apr 3, 2011 at 3:14 AM, Michael Matz m...@suse.de wrote:
 Hi,

 On Thu, 31 Mar 2011, Richard Guenther wrote:

  In the meanwhile, is the below version okay?

 If it bootstraps  tests ok then yes.  The java parts look obvious.

 So, we indeed can't remove the other calls to
 canonicalize_constructor_val, because of local ctors.  And fortran has a
 similar problem with java.  Instead of fixing up all these places of
 resetting cfun (where otherwise the frontends don't deal at all with it,
 it's mostly just set from the various cgraph routines), I decided to
 simply clear this at the appropriate place in
 cgraph_finalize_compilation_unit.

 Regstrapping in progress again.  Still okay if that works?


 Ciao,
 Michael.
 --
        * cgraphbuild.c (record_reference): Canonicalize constructor
        values.


 Index: cgraphbuild.c
 ===
 --- cgraphbuild.c.orig  2011-04-03 11:28:45.0 +0200
 +++ cgraphbuild.c       2011-04-03 11:31:21.0 +0200
 @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
   tree decl;
   struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;

 +  t = canonicalize_constructor_val (t);
 +  if (!t)
 +    t = *tp;
 +  else if (t != *tp)
 +    *tp = t;
 +
   switch (TREE_CODE (t))
     {
     case VAR_DECL:

 This change caused:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440


This avoids  canonicalizing constructor values for address conversion
if Pmode != ptr_mode.  OK for trunk?

Thanks.

-- 
H.J.

2011-04-04  H.J. Lu  hongjiu...@intel.com

PR middle-end/48440
* cgraphbuild.c (record_reference): Don't canonicalize constructor
values for address conversion if Pmode != ptr_mode.
2011-04-04  H.J. Lu  hongjiu...@intel.com

PR middle-end/48440
* cgraphbuild.c (record_reference): Don't canonicalize constructor
values for address conversion if Pmode != ptr_mode.

diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
index 3948cf6..8ba7864 100644
--- a/gcc/cgraphbuild.c
+++ b/gcc/cgraphbuild.c
@@ -49,15 +49,20 @@ struct record_reference_ctx
 static tree
 record_reference (tree *tp, int *walk_subtrees, void *data)
 {
-  tree t = *tp;
+  tree t = *tp, tc;
   tree decl;
   struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
 
-  t = canonicalize_constructor_val (t);
-  if (!t)
-t = *tp;
-  else if (t != *tp)
-*tp = t;
+  tc = canonicalize_constructor_val (t);
+  if (tc
+   tc != t
+   !(Pmode != ptr_mode
+   TREE_CODE (tc) == ADDR_EXPR
+   TREE_CODE (t) == CONVERT_EXPR))
+{
+  *tp = tc;
+  t = tc;
+}
 
   switch (TREE_CODE (t))
 {