[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.1.3


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

[Bug middle-end/31150] New: [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[pinskia at gcc dot gnu dot org]

Take:
int main()
{
  char str[2][34] = {"a","b"};

  __builtin_puts(str[0]);

  return 0;
}

In 4.0.2, we used to get:
  static char C.0[2][34] = {"a", "b"};
      char str[2][34];

      str = C.0;
      __builtin_puts (&str[0][0]);


But in 4.1.1 and above, we get:
  char str[2][34];

  str = {};
  str[0] = "a";
  str[1] = "b";

Which causes us to first zero out the array and then do load/stores on the
stack.


-- 
           Summary: [4.1/4.2/4.3 Regression] Not promoting an whole array to
                    be static const
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pinskia at gcc dot gnu dot org


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[mmitchel at gcc dot gnu dot org]

-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2007-04-16 00:56 -------
This was introduced by the fix for PR 21478.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |21478


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[eres at il dot ibm dot com]

------- Comment #2 from eres at il dot ibm dot com  2007-06-25 12:31 -------
I would like to be assigned to this bug.

Thanks,
Revital


-- 


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |eres at il dot ibm dot com
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-06-25 14:35:33
               date|                            |


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[eres at il dot ibm dot com]

------- Comment #3 from eres at il dot ibm dot com  2007-06-26 07:42 -------
Created an attachment (id=13791)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13791&action=view)
fix PR31150

Attached is a patch to initialize the scalar elmenets of the array
which should fix this problem. 

char str[2][34] = {"a","b"};
==>
str = {};
str[0][0] = 97;
str[1][0] = 98;

I appreciate any comments.

Thanks,
Revital


-- 


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[patchapp at dberlin dot org]

------- Comment #4 from patchapp at dberlin dot org  2007-06-28 05:50 -------
Subject: Bug number PR31150

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is
http://gcc.gnu.org/ml/gcc-patches/2007-06/msg02037.html


-- 


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[eres at il dot ibm dot com]

------- Comment #5 from eres at il dot ibm dot com  2007-06-28 11:55 -------
(Form off-line discussion with Richard Guenther)

For- 

char str[2][16] = {"thisis16charslo","thisis16charslo"};

On ppc64 we will get -

static char C.0[2][16] = {"thisis16charslo", "thisis16charslo"};

while on x86_64 - 

str[0] = "thisis16charslo";
str[1] = "thisis16charslo";

and this patch makes this output much worse (although it should be
applied only on sparse data)


-- 


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

[Bug middle-end/31150] [4.1/4.2/4.3 Regression] Not promoting an whole array to be static const

[jakub at gcc dot gnu dot org]

------- Comment #6 from jakub at gcc dot gnu dot org  2007-08-23 20:36 -------
It is unfortunate that gimple can only initialize the whole array, unless
__builtin_memcpy is used.  Unfortunately __builtin_memcpy has a different
drawback - the var will be forced to be TREE_ADDRESSABLE even when it otherwise
wouldn't have to be.
So, we have the option to add some flag to count_type_elements etc., so that
they will behave one way for expr.c etc. and one way for the gimplifier - there
it would need to count whole arrays as units if they are initialized from
string
literals, etc.
The other option is IMHO to hint during expansion expand_assignment that
the memory is already cleared by previous var = {} and so it can avoid storing
zeros.  Or teach some RTL pass to remove the redundant zero stores.  If
the big clear_storage is done through clearing by pieces, DSE will already
handle this to some extent (will remove redundant stores from the big
clear_storage), but if it is done through a special insn (like rep stosb) or
through memset call, nothing will remove it.

While looking at this, I have noticed that a simple
struct A { char c[10]; };

void
foo (void)
{
  struct A a = { "abcdefghi" };
  baz (&a);
}

void
bar (void)
{
  struct A a;
  __builtin_strcpy (&a.c[0], "abcdefghi");
  baz (&a);
}

is not as efficient as it ought to be in the foo function - while strcpy/memcpy
can and in this case will use store_by_pieces, store_expr will always emit the
string literal to memory and then emit a block move from it to target followed
optionally by clear_storage of the rest of the memory.
I'm testing a patch to improve this ATM.


-- 


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

[Bug middle-end/31150] [4.1/4.2/4.3/4.4 Regression] Not promoting an whole array to be static const

[pinskia at gcc dot gnu dot org]

------- Comment #7 from pinskia at gcc dot gnu dot org  2008-07-03 23:22 -------
(In reply to comment #4)
> http://gcc.gnu.org/ml/gcc-patches/2007-06/msg02037.html

This patch causes the following simple testcase to fail as we are stopping at
the first NULL character:
int main()
{
  char str[2][34] = {"a\0c","b"};

  if (str[0][2] != 'c')
    __builtin_abort ();

  return 0;
}


-- 


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

[Bug middle-end/31150] [4.2/4.3/4.4 Regression] Not promoting an whole array to be static const

[jsm28 at gcc dot gnu dot org]

------- Comment #8 from jsm28 at gcc dot gnu dot org  2008-07-04 22:00 -------
Closing 4.1 branch.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.1/4.2/4.3/4.4 Regression]|[4.2/4.3/4.4 Regression] Not
                   |Not promoting an whole array|promoting an whole array to
                   |to be static const          |be static const
   Target Milestone|4.1.3                       |4.2.5


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

[Bug middle-end/31150] [4.2/4.3/4.4 Regression] Not promoting an whole array to be static const

[eres at il dot ibm dot com]

------- Comment #9 from eres at il dot ibm dot com  2008-07-06 05:29 -------
Following http://gcc.gnu.org/ml/gcc/2008-07/msg00104.html
I would like to ask to be unassign from this bug.

Thanks


-- 


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

[Bug middle-end/31150] [4.2/4.3/4.4 Regression] Not promoting an whole array to be static const

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|eres at il dot ibm dot com  |unassigned at gcc dot gnu
                   |                            |dot org
             Status|ASSIGNED                    |NEW


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

[Bug middle-end/31150] [4.2/4.3/4.4 Regression] Not promoting an whole array to be static const

[jakub at gcc dot gnu dot org]

------- Comment #10 from jakub at gcc dot gnu dot org  2008-12-22 23:42 -------
Subject: Bug 31150

Author: jakub
Date: Mon Dec 22 23:41:17 2008
New Revision: 142892

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=142892
Log:
        PR middle-end/31150
        * dse.c (struct store_info): Add const_rhs field.
        (clear_rhs_from_active_local_stores): Clear also const_rhs.
        (record_store): Try also cselib_expand_value_rtx to get a constant.
        (find_shift_sequence, get_stored_val): Use const_rhs instead of
        rhs if worthwhile.
        * cselib.c (cselib_record_sets): If !cselib_record_memory and
        there is just one set from read-only MEM, look at REG_EQUAL or
        REG_EQUIV note.

        * dse.c (struct store_info): Add redundant_reason field.
        (record_store): When storing the same constant as has been
        stored by an earlier store, set redundant_reason field
        to the earlier store's insn_info_t.  Don't delete cannot_delete
        insns.
        (find_shift_sequence): Remove read_info argument, add read_mode
        and require_cst arguments.  Return early if require_cst and
        constant wouldn't be returned.
        (get_stored_val): New function.
        (replace_read): Use it.
        (scan_insn): Put even cannot_delete insns with exactly 1 store
        into active_local_stores.
        (dse_step1): Don't delete cannot_delete insns.  Remove redundant
        constant stores if contains_cselib_groups and earlier store storing
        the same value hasn't been eliminated.
        (dse_step6): Renamed to dse_step7.  New function.
        (dse_step7): Renamed from dse_step6.
        (rest_of_handle_dse): Call dse_step6 and dse_step7 at the end.
        * cselib.c (cselib_expand_value_rtx): Don't wrap CONST_INTs
        into CONST unless really necessary.  Handle SUBREG, unary,
        ternary, bitfield and compares specially, to be able to simplify
        operations on constants.
        (expand_loc): Try to optimize LO_SUM.

        * dse.c (get_call_args): New function.
        (scan_insn): Don't handle BUILT_IN_BZERO.  For memset, attempt
        to get call arguments and if successful and both len and val are
        constants, handle the call as (mem:BLK) (const_int) store.

        * dse.c (struct store_info): Add is_large bool field, change
        positions_needed into a union of a bitmask and bitmap + count.
        (free_store_info): Free bitmap if is_large.
        (set_usage_bits): Don't look at stores where
        offset + width >= MAX_OFFSET.
        (set_position_unneeded, set_all_positions_unneeded,
        any_positions_needed_p, all_positions_needed_p): New static inline
        functions.
        (record_store): Handle BLKmode stores of CONST_INT, if
        MEM_SIZE is set on the MEM.  Use the new positions_needed
        accessor inlines.
        (replace_read): Handle reads from BLKmode CONST_INT stores.
        (check_mem_read_rtx): Use all_positions_needed_p function.
        (dse_step1): Free large positions_needed bitmaps and clear is_large.

        * dse.c (struct store_info): Change begin and end types to
        HOST_WIDE_INT.

        * dse.c (record_store): Fix check for unused store.

        * expr.c (block_clear_fn): No longer static.
        * expr.h (block_clear_fn): Declare.
        * dse.c (scan_insn): Memset and bzero can just read their
        arguments.

        * gcc.c-torture/execute/20081218-1.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/20081218-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cselib.c
    trunk/gcc/dse.c
    trunk/gcc/expr.c
    trunk/gcc/expr.h
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug middle-end/31150] [4.2/4.3/4.4 Regression] Not promoting an whole array to be static const

[jakub at gcc dot gnu dot org]

------- Comment #11 from jakub at gcc dot gnu dot org  2008-12-22 23:45 -------
Fixed on the trunk.  Won't fix for older releases.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.2.5                       |4.4.0


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