Bug 33344 - if(!z) {a.b = x->b + 1; z = &a; } else if (!x) { a.b = z->b+1; x = &a; } use z->b and x->b; is not optimized
Summary: if(!z) {a.b = x->b + 1; z = &a; } else if (!x) { a.b = z->b+1; x = &a; } use ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P3 enhancement
Target Milestone: 4.5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on: 37892
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-08 00:02 UTC by Andrew Pinski
Modified: 2009-04-03 11:40 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-09-08 13:00:10


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Pinski 2007-09-08 00:02:18 UTC
Testcase:
struct f { int a; };
int g(struct f *b, struct f *c)
{
  struct f g;
  if (!b) {
    b = &g;
    b->a = c->a + 1;
  }
  else if (!c) {
    c = &g;
    c->a = b->a + 1;
  }
  return c->a + b->a;
}

The best way to optimize this is:
struct f { int a; };
int g(struct f *b, struct f *c)
{
  int ba, ca;
  if (!b) {
    ca = c->a;
    ba = ca + 1;
  }
  else if (!c) {
    ba = b->a;
    ca = ba + 1;
  }
  else {
    ca = c->a;
    ba = b->a;
  }
  return ca + ba;
}

Which avoids the LHS (load-hit-store hazzard) issue on the Cell.
Comment 1 Richard Biener 2007-09-08 13:00:10 UTC
It's currently not optimized because the simple phiprop does neither handle
component references nor inserts loads from pointers.

See the thread starting at
http://gcc.gnu.org/ml/gcc-patches/2007-04/msg01075.html
on how to teach PRE to do a small set of possible transformations like this.
Comment 2 Richard Biener 2007-09-08 17:49:45 UTC
So the following brute force approach will make PRE to do insertion for _all_
loads.  It produces then

g (b, c)
{
  int prephitmp.15;
  int prephitmp.10;
  struct f g;

<bb 2>:
  if (b == 0B)
    goto <bb 3>;
  else
    goto <bb 4>;

<bb 3>:
  prephitmp.10 = c->a + 1;
  g.a = prephitmp.10;
  prephitmp.15 = c->a;
  goto <bb 7>;

<bb 4>:
  if (c == 0B)
    goto <bb 6>;
  else
    goto <bb 5>;

<bb 5>:
  prephitmp.15 = c->a;
  prephitmp.10 = b->a;
  goto <bb 7>;

<bb 6>:
  prephitmp.15 = b->a + 1;
  g.a = prephitmp.15;
  prephitmp.10 = b->a;

<bb 7>:
  return prephitmp.10 + prephitmp.15;

}


Index: tree-ssa-pre.c
===================================================================
--- tree-ssa-pre.c      (revision 128275)
+++ tree-ssa-pre.c      (working copy)
@@ -2598,6 +2598,7 @@ do_regular_insertion (basic_block block,
          bool by_some = false;
          bool cant_insert = false;
          bool all_same = true;
+         bool is_load = false;
          tree first_s = NULL;
          edge pred;
          basic_block bprime;
@@ -2614,6 +2615,10 @@ do_regular_insertion (basic_block block,
              continue;
            }
 
+         if (TREE_CODE (expr) == INDIRECT_REF
+             || handled_component_p (expr))
+           is_load = true;
+
          avail = XCNEWVEC (tree, last_basic_block);
          FOR_EACH_EDGE (pred, ei, block->preds)
            {
@@ -2672,7 +2677,7 @@ do_regular_insertion (basic_block block,
             already existing along every predecessor, and
             it's defined by some predecessor, it is
             partially redundant.  */
-         if (!cant_insert && !all_same && by_some)
+         if (!cant_insert && !all_same && (by_some || is_load))
            {
              if (insert_into_preds_of_block (block, get_expression_id (expr),
                                              avail))


The is_load should be really walk leading exprs looking for an inner
indirect_ref.  And edges should be walked to look for at least one
addr_expr in the phi arguments affected.
Comment 3 Richard Biener 2007-09-08 17:51:24 UTC
Note that basic cleanups after PRE are missing, for example FRE of inserted loads.
And of course it makes code larger.
Comment 4 Richard Biener 2009-04-03 11:40:50 UTC
Fixed with the alias-improvements branch merge.

g (struct f * b, struct f * c)
{
  int prephitmp.20;
  int prephitmp.19;

<bb 2>:
  if (b == 0B)
    goto <bb 3>;
  else
    goto <bb 4>;

<bb 3>:
  prephitmp.19 = c->a;
  prephitmp.20 = prephitmp.19 + 1;
  goto <bb 7>;

<bb 4>:
  if (c == 0B)
    goto <bb 6>;
  else
    goto <bb 5>;

<bb 5>:
  prephitmp.19 = c->a;
  prephitmp.20 = b->a;
  goto <bb 7>;

<bb 6>:
  prephitmp.20 = b->a;
  prephitmp.19 = prephitmp.20 + 1;

<bb 7>:
  return prephitmp.20 + prephitmp.19;

}