public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg
@ 2012-01-19  2:22 jakub at gcc dot gnu.org
  2012-01-19  4:17 ` [Bug middle-end/51895] " bergner at gcc dot gnu.org
                   ` (18 more replies)
  0 siblings, 19 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-19  2:22 UTC (permalink / raw)
  To: gcc-bugs

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

             Bug #: 51895
           Summary: [4.7 Regression] ICE in simplify_subreg
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jakub@gcc.gnu.org
                CC: bergner@gcc.gnu.org
            Target: powerpc64-linux


struct S
{
  long a;
  char b;
  S () : a (0), b (0) {}
  bool baz ();
};

__attribute__((noinline)) static bool
bar (S x, S y)
{
  y = x;
  return y.baz ();
}

bool
foo (S x)
{
  S y;
  return bar (x, y);
}

ICEs at -O2 -m64 with:
rh782868.ii: In function ‘bool foo(S)’:
rh782868.ii:20:19: internal compiler error: in simplify_subreg, at
simplify-rtx.c:5420
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
@ 2012-01-19  4:17 ` bergner at gcc dot gnu.org
  2012-01-19  8:19 ` jakub at gcc dot gnu.org
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: bergner at gcc dot gnu.org @ 2012-01-19  4:17 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 from Peter Bergner <bergner at gcc dot gnu.org> 2012-01-19 02:42:44 UTC ---
We ICE when we try and do a BLK move from a reg:TI.  In GCC 4.6, we don't see
this, since the args[i].tree_value is different:

(gdb-gcc4.6) ptree args[i].tree_value
 <var_decl 0xfffaa5703c0 Ptr1
    type <record_type 0xfffafd6f960 SDValue sizes-gimplified needs-constructing
type_1 type_5 type_6 TI
        size <integer_cst 0xfffb6d50a00 constant 128>
        unit size <integer_cst 0xfffb6d50a28 constant 16>
        align 64 symtab 0 alias set 43 canonical type 0xfffafd6f960
        fields <field_decl 0xfffaec22ee8 Node type <pointer_type 0xfffafd6fab0>
            used private unsigned nonlocal decl_3 DI file t.ii line 30433 col
11
            size <integer_cst 0xfffb6d50870 constant 64>
            unit size <integer_cst 0xfffb6d50898 constant 8>
            align 64 offset_align 128
            offset <integer_cst 0xfffb6d504d8 constant 0>
            bit offset <integer_cst 0xfffb6d50fa0 constant 0> context
<record_type 0xfffafd6f960 SDValue> chain <field_decl 0xfffaec22f80 ResNo>>
context <namespace_decl 0xfffb2ef1368 llvm>
        full-name "class llvm::SDValue"
        needs-constructor X() X(constX&) this=(X&) n_parents=0 use_template=0
interface-unknown
        pointer_to_this <pointer_type 0xfffafd6fca8> reference_to_this
<reference_type 0xfffafe27818> chain <type_decl 0xfffaebe9d68 SDValue>>
    used TI file t.ii line 45352 col 9 size <integer_cst 0xfffb6d50a00 128>
unit size <integer_cst 0xfffb6d50a28 16>
    align 64 context <function_decl 0xffface9a300 FindBetterChain>
abstract_origin <parm_decl 0xfffae3bf1b8 Ptr1>
    (reg/v:TI 306 [ Ptr1 ])>

versus for gcc 4.7:

(gdb-gc4.7) ptree args[i].tree_value
 <mem_ref 0xfffa8e935a0
    type <array_type 0xfffb0cac198
        type <integer_type 0xfffb5e403f0 char sizes-gimplified public unsigned
string-flag type_6 QI
            size <integer_cst 0xfffb5d527e0 constant 8>
            unit size <integer_cst 0xfffb5d52800 constant 1>
            align 8 symtab 0 alias set -1 canonical type 0xfffb5e403f0
precision 8 min <integer_cst 0xfffb5d52860 0> max <integer_cst 0xfffb5d52880
255>
            pointer_to_this <pointer_type 0xfffb5e43288> reference_to_this
<reference_type 0xfffb5e4e700>>
        BLK
        size <integer_cst 0xfffb2c63bc0 constant 96>
        unit size <integer_cst 0xfffb2c63540 constant 12>
        align 8 symtab 0 alias set 0 canonical type 0xfffb0cac198
        domain <integer_type 0xfffb0cac0f0 type <integer_type 0xfffb5e40000
sizetype>
            DI
            size <integer_cst 0xfffb5d52600 constant 64>
            unit size <integer_cst 0xfffb5d52620 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0xfffb0cac0f0
precision 64 min <integer_cst 0xfffb5d52640 0> max <integer_cst 0xfffb2c6f020
11>>
        pointer_to_this <pointer_type 0xfffa2f99cd8>>

    arg 0 <addr_expr 0xfffa3d2a708
        type <pointer_type 0xfffa8d50b28 type <record_type 0xfffa8d505e8
SDValue>
            sizes-gimplified public unsigned type_6 DI size <integer_cst
0xfffb5d52600 64> unit size <integer_cst 0xfffb5d52620 8>
            align 64 symtab 0 alias set 2 canonical type 0xfffa8d50b28
            pointer_to_this <pointer_type 0xfffa3834cc8>>

        arg 0 <var_decl 0xfffa362ee20 Ptr1 type <record_type 0xfffa8d505e8
SDValue>
            used TI file t.ii line 45286 col 6
            size <integer_cst 0xfffb5d526a0 constant 128>
            unit size <integer_cst 0xfffb5d526c0 constant 16>
            align 64 context <function_decl 0xfffa2db2700 GatherAllAliases>
abstract_origin <parm_decl 0xfffa32f8ff8 Ptr1>
            (reg/v:TI 279 [ Ptr1 ])>>
    arg 1 <integer_cst 0xfffa9014f00 type <pointer_type 0xfffa8d50b28> constant
0>>

That difference forces us down different paths starting at this
load_register_parameters() test:

   else if (TYPE_MODE (TREE_TYPE (args[i].tree_value)) == BLKmode)

For 4.6, the mode is a TImode and for 4.7, it's a BLKmode.  The following patch
seems to fix it:

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c (revision 183280)
+++ gcc/emit-rtl.c (working copy)
@@ -1401,6 +1401,9 @@ operand_subword (rtx op, unsigned int of
  return replace_equiv_address (new_rtx, XEXP (new_rtx, 0));
     }

+  if (REG_P (op) && mode == BLKmode)
+    mode = GET_MODE (op);
+
   /* Rest can be handled by simplify_subreg.  */
   return simplify_gen_subreg (word_mode, op, mode, (offset * UNITS_PER_WORD));
 }


I'm bootstraping/regtesting this patch now.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
  2012-01-19  4:17 ` [Bug middle-end/51895] " bergner at gcc dot gnu.org
@ 2012-01-19  8:19 ` jakub at gcc dot gnu.org
  2012-01-19 10:16 ` rguenth at gcc dot gnu.org
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-19  8:19 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jamborm at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-19 07:33:07 UTC ---
This starts with eipa_sra.  It changes a S argument (which has TImode
TYPE_MODE) into char [9] (with BLKmode)) and then on both caller and callee
side we have on one side a BLKmode type and on the other side a BLKmode MEM_REF
with pointer to TImode on the second MEM_REF operand.
I wonder why it does this, instead of just using type S, and if it really has
to for some reason, why it can't at least make sure it has the same TYPE_MODE.
Changing a TImode argument to a BLKmode argument doesn't look at least like a
good optimization.

Or the bug is in the MEM_REF expansion, which expands a BLKmode MEM_REF into a
TImode reg:
                bftype = TREE_TYPE (base);
                if (TYPE_MODE (TREE_TYPE (exp)) != BLKmode)
                  bftype = TREE_TYPE (exp);
                return expand_expr (build3 (BIT_FIELD_REF, bftype,
                                            base,
                                            TYPE_SIZE (TREE_TYPE (exp)),
                                            bit_offset),
                                    target, tmode, modifier);
base here is TImode (x PARM_DECL), but exp is BLKmode, so this returns a TImode
pseudo.  Shouldn't it store it into a BLKmode temporary and return that MEM
instead?


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
  2012-01-19  4:17 ` [Bug middle-end/51895] " bergner at gcc dot gnu.org
  2012-01-19  8:19 ` jakub at gcc dot gnu.org
@ 2012-01-19 10:16 ` rguenth at gcc dot gnu.org
  2012-01-19 10:47 ` jakub at gcc dot gnu.org
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-19 10:16 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-01-19
   Target Milestone|---                         |4.7.0
     Ever Confirmed|0                           |1

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-19 10:06:38 UTC ---
(In reply to comment #2)
> This starts with eipa_sra.  It changes a S argument (which has TImode
> TYPE_MODE) into char [9] (with BLKmode)) and then on both caller and callee
> side we have on one side a BLKmode type and on the other side a BLKmode MEM_REF
> with pointer to TImode on the second MEM_REF operand.
> I wonder why it does this, instead of just using type S, and if it really has
> to for some reason, why it can't at least make sure it has the same TYPE_MODE.
> Changing a TImode argument to a BLKmode argument doesn't look at least like a
> good optimization.
> 
> Or the bug is in the MEM_REF expansion, which expands a BLKmode MEM_REF into a
> TImode reg:
>                 bftype = TREE_TYPE (base);
>                 if (TYPE_MODE (TREE_TYPE (exp)) != BLKmode)
>                   bftype = TREE_TYPE (exp);
>                 return expand_expr (build3 (BIT_FIELD_REF, bftype,
>                                             base,
>                                             TYPE_SIZE (TREE_TYPE (exp)),
>                                             bit_offset),
>                                     target, tmode, modifier);
> base here is TImode (x PARM_DECL), but exp is BLKmode, so this returns a TImode
> pseudo.  Shouldn't it store it into a BLKmode temporary and return that MEM
> instead?

Using a BIT_FIELD_REF looked most convenient.  Using extract_bit_field
may also be an option (which I suppose is what the above ends up doing?)


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2012-01-19 10:16 ` rguenth at gcc dot gnu.org
@ 2012-01-19 10:47 ` jakub at gcc dot gnu.org
  2012-01-19 12:47 ` jakub at gcc dot gnu.org
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-19 10:47 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-19 10:16:07 UTC ---
(In reply to comment #3)
> >                 bftype = TREE_TYPE (base);
> >                 if (TYPE_MODE (TREE_TYPE (exp)) != BLKmode)
> >                   bftype = TREE_TYPE (exp);
> >                 return expand_expr (build3 (BIT_FIELD_REF, bftype,
> >                                             base,
> >                                             TYPE_SIZE (TREE_TYPE (exp)),
> >                                             bit_offset),
> >                                     target, tmode, modifier);
> > base here is TImode (x PARM_DECL), but exp is BLKmode, so this returns a TImode
> > pseudo.  Shouldn't it store it into a BLKmode temporary and return that MEM
> > instead?
> 
> Using a BIT_FIELD_REF looked most convenient.  Using extract_bit_field
> may also be an option (which I suppose is what the above ends up doing?)

I think if exp is BLKmode, then we don't want to do a BIT_FIELD_REF nor
extract_bit_field.  We IMHO need to store base into a temporary and just adjust
the MEM.  Or extract the bit field and then store it into a temporary and
adjust, but the former looks easier.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2012-01-19 10:47 ` jakub at gcc dot gnu.org
@ 2012-01-19 12:47 ` jakub at gcc dot gnu.org
  2012-01-19 13:12 ` rguenth at gcc dot gnu.org
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-19 12:47 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-19 12:33:55 UTC ---
Created attachment 26377
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26377
gcc47-pr51895.patch

Untested patch that attempts to fix BLKmode MEM_REF expansion with
non-DECL_ADDRESSABLE non-BLKmode base.  It creates abysmal code, so IMNSHO
eipa_sra should be fixed not to do this.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2012-01-19 12:47 ` jakub at gcc dot gnu.org
@ 2012-01-19 13:12 ` rguenth at gcc dot gnu.org
  2012-01-19 13:32 ` jakub at gcc dot gnu.org
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-19 13:12 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-19 13:03:14 UTC ---
(In reply to comment #5)
> Created attachment 26377 [details]
> gcc47-pr51895.patch
> 
> Untested patch that attempts to fix BLKmode MEM_REF expansion with
> non-DECL_ADDRESSABLE non-BLKmode base.  It creates abysmal code, so IMNSHO
> eipa_sra should be fixed not to do this.

Hm, can't we do better using extract_bit_field?  I mean, it definitely
should work to do any BIT_FIELD_REF on an rvalue, even if it is a register.
The patch from comment #1 doesn't look completely wrong, it just seems that
the caller should have catered for using the mode of the reg.  The docs
of operand_subword also say 'MODE is the mode of OP in case it is a CONST_INT'
so MODE should be irrelevant if REG_P (op) ...

Seems to be a tricky area, but using a stack temporary looks like overkill.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2012-01-19 13:12 ` rguenth at gcc dot gnu.org
@ 2012-01-19 13:32 ` jakub at gcc dot gnu.org
  2012-01-19 13:38 ` rguenth at gcc dot gnu.org
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-19 13:32 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-19 13:11:13 UTC ---
(In reply to comment #6)
> > Untested patch that attempts to fix BLKmode MEM_REF expansion with
> > non-DECL_ADDRESSABLE non-BLKmode base.  It creates abysmal code, so IMNSHO
> > eipa_sra should be fixed not to do this.
> 
> Hm, can't we do better using extract_bit_field?  I mean, it definitely
> should work to do any BIT_FIELD_REF on an rvalue, even if it is a register.
> The patch from comment #1 doesn't look completely wrong, it just seems that
> the caller should have catered for using the mode of the reg.  The docs
> of operand_subword also say 'MODE is the mode of OP in case it is a CONST_INT'
> so MODE should be irrelevant if REG_P (op) ...
> 
> Seems to be a tricky area, but using a stack temporary looks like overkill.

We don't have BLKmode pseudos, the only thing that can be BLKmode is MEM.  So
I'm afraid we can't avoid that.  E.g. VIEW_CONVERT_EXPR of a TImode value to
BLKmode value would be expanded by spilling the TImode value to a stack
temporary and adjust_address it to BLKmode too.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2012-01-19 13:32 ` jakub at gcc dot gnu.org
@ 2012-01-19 13:38 ` rguenth at gcc dot gnu.org
  2012-01-19 14:07 ` jakub at gcc dot gnu.org
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-19 13:38 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-19 13:17:25 UTC ---
For example (untested, and probably completely bogus):

Index: expr.c
===================================================================
--- expr.c      (revision 183296)
+++ expr.c      (working copy)
@@ -1517,7 +1517,9 @@ move_block_to_reg (int regno, rtx x, int

   for (i = 0; i < nregs; i++)
     emit_move_insn (gen_rtx_REG (word_mode, regno + i),
-                   operand_subword_force (x, i, mode));
+                   operand_subword_force (x, i,
+                                          CONSTANT_P (x)
+                                          ? mode : GET_MODE (x)));
 }

 /* Copy all or part of a BLKmode value X out of registers starting at REGNO.

As written elsewhere operand_subword* should be deprecated ... so maybe
we can use sth else in move_block_to_reg?

The same effect could be achieved by changing operand_subword to adhere
to its documentation and ignore mode if op is !CONST_INT_P ... thus

Index: emit-rtl.c
===================================================================
--- emit-rtl.c  (revision 183296)
+++ emit-rtl.c  (working copy)
@@ -1367,7 +1367,7 @@ paradoxical_subreg_p (const_rtx x)
 rtx
 operand_subword (rtx op, unsigned int offset, int validate_address, enum
machine_mode mode)
 {
-  if (mode == VOIDmode)
+  if (!CONST_INT_P (op))
     mode = GET_MODE (op);

   gcc_assert (mode != VOIDmode);

Both fix the testcase.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2012-01-19 13:38 ` rguenth at gcc dot gnu.org
@ 2012-01-19 14:07 ` jakub at gcc dot gnu.org
  2012-01-19 16:28 ` ebotcazou at gcc dot gnu.org
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-19 14:07 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-19 13:55:31 UTC ---
Peter's patch does as well.  The thing is that we still do wrong expand_expr on
that kind of MEM_EXPR, and I don't see anything that would prevent such
MEM_EXPRs in all kinds of other contexts, not just in the loading of the
parameters.
And each of the would need hacks to cope with a BLKmode expression surprisingly
being expanded to something that doesn't have BLKmode.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2012-01-19 14:07 ` jakub at gcc dot gnu.org
@ 2012-01-19 16:28 ` ebotcazou at gcc dot gnu.org
  2012-01-20  9:42 ` jakub at gcc dot gnu.org
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2012-01-19 16:28 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-19 16:13:55 UTC ---
> I wonder why it does this, instead of just using type S, and if it really has
> to for some reason, why it can't at least make sure it has the same TYPE_MODE.
> Changing a TImode argument to a BLKmode argument doesn't look at least like a
> good optimization.

I concur, BLKmode means spilling to memory at some point, so this looks like a
clear pessimization to me.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2012-01-19 16:28 ` ebotcazou at gcc dot gnu.org
@ 2012-01-20  9:42 ` jakub at gcc dot gnu.org
  2012-01-20  9:54 ` ebotcazou at gcc dot gnu.org
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-20  9:42 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-20 09:07:54 UTC ---
(In reply to comment #10)
> > I wonder why it does this, instead of just using type S, and if it really has
> > to for some reason, why it can't at least make sure it has the same TYPE_MODE.
> > Changing a TImode argument to a BLKmode argument doesn't look at least like a
> > good optimization.
> 
> I concur, BLKmode means spilling to memory at some point, so this looks like a
> clear pessimization to me.

Sure, but I believe just that eipa_sra is what is causing this pessimization by
doing a bad decision.
Anyway, if you prefer one of the other 3 patches (for now?), which one it is? 
Or some other place to handle it?


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2012-01-20  9:42 ` jakub at gcc dot gnu.org
@ 2012-01-20  9:54 ` ebotcazou at gcc dot gnu.org
  2012-01-20 14:36 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2012-01-20  9:54 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-20 09:41:41 UTC ---
> Sure, but I believe just that eipa_sra is what is causing this pessimization by
> doing a bad decision.

Precisely, so why not change that?

> Anyway, if you prefer one of the other 3 patches (for now?), which one it is? 

Yours looks less hackish for sure. :-)


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2012-01-20  9:54 ` ebotcazou at gcc dot gnu.org
@ 2012-01-20 14:36 ` rguenth at gcc dot gnu.org
  2012-01-20 14:38 ` rguenth at gcc dot gnu.org
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-20 14:36 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-20 14:18:45 UTC ---
I'll look at the IPA-SRA issue.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2012-01-20 14:36 ` rguenth at gcc dot gnu.org
@ 2012-01-20 14:38 ` rguenth at gcc dot gnu.org
  2012-01-20 15:30 ` rguenth at gcc dot gnu.org
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-20 14:38 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|powerpc64-linux             |powerpc64-linux, i?86-*-*

--- Comment #14 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-20 14:20:17 UTC ---
Same ICE on i?86-linux btw.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2012-01-20 14:38 ` rguenth at gcc dot gnu.org
@ 2012-01-20 15:30 ` rguenth at gcc dot gnu.org
  2012-01-20 17:23 ` jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-20 15:30 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-20 14:40:09 UTC ---
Testing

@@ -3891,6 +3853,8 @@ decide_one_param_reduction (struct acces
     {
       by_ref = false;
       agg_size = cur_parm_size;
+      if (DECL_MODE (parm) != BLKmode)
+    return 0;
     }

   if (dump_file)

as passing a parameter by value in non-BLKmode (on the PARM_DECL) should
be good evidence to not split it down further (in this case we have
TYPE_SIZE != MODE_SIZE as well, but that's another story).

Hopefully DECL_MODE on the PARM_DECL is good enough and reflects actual
argument passing closely enough ... ?


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2012-01-20 15:30 ` rguenth at gcc dot gnu.org
@ 2012-01-20 17:23 ` jakub at gcc dot gnu.org
  2012-01-23 12:37 ` rguenth at gcc dot gnu.org
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-20 17:23 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-20 16:49:52 UTC ---
Created attachment 26396
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26396
l.ii

Note that the BLKmode MEM_REF with non-BLKmode base expansion can be triggered
also without SRA, e.g. this (distilled from locale-inst.cc) reaches the code
on i?86-linux with -m32 -O2 -fno-ipa-sra -fno-tree-sra.


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (15 preceding siblings ...)
  2012-01-20 17:23 ` jakub at gcc dot gnu.org
@ 2012-01-23 12:37 ` rguenth at gcc dot gnu.org
  2012-01-26 14:58 ` jakub at gcc dot gnu.org
  2012-01-26 15:03 ` jakub at gcc dot gnu.org
  18 siblings, 0 replies; 20+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-23 12:37 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #17 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-23 11:59:57 UTC ---
Author: rguenth
Date: Mon Jan 23 11:59:53 2012
New Revision: 183429

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183429
Log:
2012-01-23  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/51895
    * tree-sra.c (decide_one_param_reduction): Avoid sub-optimal
    parameter decomposition into BLKmode components.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-sra.c


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (16 preceding siblings ...)
  2012-01-23 12:37 ` rguenth at gcc dot gnu.org
@ 2012-01-26 14:58 ` jakub at gcc dot gnu.org
  2012-01-26 15:03 ` jakub at gcc dot gnu.org
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-26 14:58 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-26 14:09:34 UTC ---
Author: jakub
Date: Thu Jan 26 14:09:29 2012
New Revision: 183560

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183560
Log:
    PR middle-end/51895
    * expr.c (expand_expr_real_1): Handle BLKmode MEM_REF of
    non-addressable non-BLKmode base correctly.

    * g++.dg/opt/pr51895.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/opt/pr51895.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expr.c
    trunk/gcc/testsuite/ChangeLog


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

* [Bug middle-end/51895] [4.7 Regression] ICE in simplify_subreg
  2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
                   ` (17 preceding siblings ...)
  2012-01-26 14:58 ` jakub at gcc dot gnu.org
@ 2012-01-26 15:03 ` jakub at gcc dot gnu.org
  18 siblings, 0 replies; 20+ messages in thread
From: jakub at gcc dot gnu.org @ 2012-01-26 15:03 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-26 14:11:24 UTC ---
Fixed.


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

end of thread, other threads:[~2012-01-26 14:12 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-19  2:22 [Bug middle-end/51895] New: [4.7 Regression] ICE in simplify_subreg jakub at gcc dot gnu.org
2012-01-19  4:17 ` [Bug middle-end/51895] " bergner at gcc dot gnu.org
2012-01-19  8:19 ` jakub at gcc dot gnu.org
2012-01-19 10:16 ` rguenth at gcc dot gnu.org
2012-01-19 10:47 ` jakub at gcc dot gnu.org
2012-01-19 12:47 ` jakub at gcc dot gnu.org
2012-01-19 13:12 ` rguenth at gcc dot gnu.org
2012-01-19 13:32 ` jakub at gcc dot gnu.org
2012-01-19 13:38 ` rguenth at gcc dot gnu.org
2012-01-19 14:07 ` jakub at gcc dot gnu.org
2012-01-19 16:28 ` ebotcazou at gcc dot gnu.org
2012-01-20  9:42 ` jakub at gcc dot gnu.org
2012-01-20  9:54 ` ebotcazou at gcc dot gnu.org
2012-01-20 14:36 ` rguenth at gcc dot gnu.org
2012-01-20 14:38 ` rguenth at gcc dot gnu.org
2012-01-20 15:30 ` rguenth at gcc dot gnu.org
2012-01-20 17:23 ` jakub at gcc dot gnu.org
2012-01-23 12:37 ` rguenth at gcc dot gnu.org
2012-01-26 14:58 ` jakub at gcc dot gnu.org
2012-01-26 15:03 ` jakub at gcc dot gnu.org

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