Bug 43571 - domwalk sucks
Summary: domwalk sucks
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.5.0
: P3 enhancement
Target Milestone: 4.6.0
Assignee: Richard Biener
URL:
Keywords: missed-optimization
Depends on:
Blocks: 43984
  Show dependency treegraph
 
Reported: 2010-03-29 14:33 UTC by Richard Biener
Modified: 2010-05-06 09:09 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-03-29 14:35:46


Attachments
testcase (4.58 KB, text/plain)
2010-03-29 14:34 UTC, Richard Biener
Details
patch (795 bytes, patch)
2010-03-29 14:35 UTC, Richard Biener
Details | Diff
fixed patch (839 bytes, patch)
2010-04-16 13:52 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2010-03-29 14:33:26 UTC
DOM doesn't walk dominator sons in useful order to fixup complete unrolling.
Especially diamond CFG patters are walked in random order instead of first
visiting preds of the merge block.
Comment 1 Richard Biener 2010-03-29 14:34:21 UTC
Created attachment 20248 [details]
testcase

-O3 -funroll-loops -fno-tree-vrp

(VRP messes up loop structure)
Comment 2 Richard Biener 2010-03-29 14:35:33 UTC
Created attachment 20249 [details]
patch
Comment 3 Richard Biener 2010-03-29 14:35:46 UTC
Mine.
Comment 4 Richard Biener 2010-03-29 14:57:20 UTC
Other issue.  DOM doesn't track non-executable BBs.  Same testcase, see how

  if (D.1574_205 > D.1636_208)
    goto <bb 22>;
  else
    goto <bb 23>;

<bb 22>:
  iftmp.5_209 = iftmp.5_206 + 1;

<bb 23>:
  # iftmp.5_210 = PHI <iftmp.5_206(21), iftmp.5_209(22)>

the condition is folded but iftmp.5_210 is not found to be constant
(optimize_stmt asks for find_taken_edge but nothing tracks edge
executability, which also could save compile-time.  record_equivalences_from_phis would need to ignore values from
non-executable edges)
Comment 5 Michael Matz 2010-03-29 15:25:57 UTC
In your patch you need to sbitmap_allocate (last_basic_block)  (instead of
n_basic_blocks).
Comment 6 Richard Biener 2010-04-16 13:52:13 UTC
Created attachment 20395 [details]
fixed patch

The testcase doesn't reproduce the problem w/o the patch anymore.
Comment 7 Richard Biener 2010-05-06 09:09:20 UTC
Subject: Bug 43571

Author: rguenth
Date: Thu May  6 09:08:57 2010
New Revision: 159100

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159100
Log:
2010-05-06  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43571
	* domwalk.c (walk_dominator_tree): Walk the dominator
	sons in more optimal order.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/domwalk.c

Comment 8 Richard Biener 2010-05-06 09:09:34 UTC
Fixed.