Bug 22532 - [4.1 Regression] We produce worse code on the mainline for a loop
Summary: [4.1 Regression] We produce worse code on the mainline for a loop
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.1.0
: P2 normal
Target Milestone: 4.1.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks: 19905
  Show dependency treegraph
 
Reported: 2005-07-17 20:50 UTC by Andrew Pinski
Modified: 2005-07-19 19:57 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-07-18 07:41:32


Attachments
patch which I need to test (1.24 KB, patch)
2005-07-18 06:43 UTC, Andrew Pinski
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Pinski 2005-07-17 20:50:07 UTC
Take the following code:
#define NUMPOINTS 20

static float opoints[NUMPOINTS];

double sqrt (double);
double f(double);

void NormalizeVectors1 (void)
{
  int i, r;
  float s, x, y, z;
  static float d = 0.0;
  d += 0.2f;
  s = d;
  for (i=0; i<NUMPOINTS; i++)
    opoints[i] = f(s);
}

Compile at -O2 and -O3 and compare.  See how we load/store now at -O3 to the static variable, d 
inside the loop.
Comment 1 Daniel Berlin 2005-07-17 21:36:16 UTC
the eustores stuff i just committed should fix this now
please try again :)
Comment 2 Andrew Pinski 2005-07-17 21:37:10 UTC
(In reply to comment #1)
> the eustores stuff i just committed should fix this now
> please try again :)
This was after eustores stuff was committed.

Comment 3 Andrew Pinski 2005-07-18 05:17:27 UTC
The reason why eustore does not remove it is because we have a loop and a PHI intbetween the load 
and store:
  #   d_15 = V_MUST_DEF <d_1>;
  d = s_2;
  #   d_18 = V_MAY_DEF <d_15>;
  f (s_9);
  #   VUSE <d_18>;
  s_16 = d;
  i_17 = i_3 + 1;

  # i_3 = PHI <i_10(0), i_17(1)>;
  # s_2 = PHI <s_8(0), s_16(1)>;
  # d_1 = PHI <d_4(0), d_18(1)>;
<L1>:;
  if (i_3 < l_11) goto <L0>; else goto <L2>;

<L2>:;
  s_12 = s_2;
  #   d_13 = V_MUST_DEF <d_1>;
  d = s_2;

See how have a PHI involved.  Maybe eustore should handle PHIs too.
Comment 4 Andrew Pinski 2005-07-18 06:43:48 UTC
Created attachment 9298 [details]
patch which I need to test

This patch which needs more comments but should be complete otherwise.	This
implements my suggesting for following PHI nodes.
It also cleans up some of the code by doing bsi_next/continue only in one
place.

ChangeLog:
(remove_useless_store_vop_expr): New function.
(remove_useless_store_p): New function.
(do_eustores): Use remove_useless_store_p.

Daniel what do you think about this patch?  If you could suggest better
function names because I could not think of better name as it is late.
Comment 5 Daniel Berlin 2005-07-18 16:04:01 UTC
Subject: Re:  [4.1 Regression] We produce
	worse code on the mainline for a loop

On Mon, 2005-07-18 at 06:43 +0000, pinskia at gcc dot gnu dot org wrote:
> ------- Additional Comments From pinskia at gcc dot gnu dot org  2005-07-18 06:43 -------
> Created an attachment (id=9298)
>  --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=9298&action=view)
> patch which I need to test
> 
> This patch which needs more comments but should be complete otherwise.	This
> implements my suggesting for following PHI nodes.
> It also cleans up some of the code by doing bsi_next/continue only in one
> place.
> 
> ChangeLog:
> (remove_useless_store_vop_expr): New function.
> (remove_useless_store_p): New function.
> (do_eustores): Use remove_useless_store_p.

Strangely, i did almost the exact same thing last night.

Among other things, you should be using FOR_EACH_PHI_ARG.

Also, remove the DECL_P checks in favor of:

+             if (! DECL_P (TREE_OPERAND (stmt, 0)))
+               mark_sym_for_renaming (SSA_NAME_VAR (kill));
+             else
+               mark_sym_for_renaming (TREE_OPERAND (stmt, 0));


which should work.

> 
> Daniel what do you think about this patch?  If you could suggest better
> function names because I could not think of better name as it is late.
> 

Comment 6 Andrew Pinski 2005-07-19 19:57:34 UTC
Fixed removing promot-statics by:
2005-07-19  Danny Berlin <dberlin@dberlin.org>
            Kenneth Zadeck <zadeck@naturalbridge.com>

        * Makefile.in: Removed tree-promote-statics.c
        * tree-promote-statics.c: Removed.
        * common.opt: Removed flag-promote-statics.
        * opts.c: Ditto.
        * passes.c: Removed tree-promote-statics pass.
        * tree-pass.h: Ditto.
        * timevar.def: Removed TV_PROMOTE_STATICS.