Patch submissions and discussion of these patches

Roger Sayle | 1 Dec 2003 03:20

[PATCH] PR opt/12628: Call reg_scan before jump bypassing


The following patch is my proposed fix for PR optimization/12628
which is an ICE-on-valid caused by an obscure combination of compiler
flags.  The jump bypassing pass calls init_alias_analysis assuming
that register information is currently correct.  Unfortunately,
with the options "-O2 -fno-expensive-optimizations -fno-rerun-loop-opt"
reg_scan isn't called after either GCSE nor loop.  This then leads
to the ICE in init_alias_analysis.

The proposed (and least invasive) solution is for toplev.c's
rest_of_handle_jump_bypass to call reg_scan itself as it needs
aliasing information.  This is consistent with most other uses
of reg_scan in toplev.c where it's called immediately before
the pass that requires it, for example, the CSE passes.  Alas
this policy is not entirely consistent, with GCSE relying on
reg_scan being called at the end of addressof.  It would be
more consistent to move reg_scan from the end of addressof to
the beginning of GCSE, but this could potentially expose a
similar failure caused by another obscure set of compiler flags.

Longer term, it would be nice if, like the CFG, we maintained
a set of flags indicating whether these data structures were
valid following an optimization pass.  This would save us from
recalculating information that hasn't changed.

Unfortunately, calling reg_scan performs a traversal of the
RTL insn stream, so we can expect a minor slow-down from this
change :<.  It isn't clear whether GCSE or loop preserve the
"reg_scan" information, so this change is safe even when the
user hasn't specified wierd command line options.  However,

(Continue reading)

headers

Geoff Keating | 1 Dec 2003 05:26

Re: [PATCH] PR opt/12628: Call reg_scan before jump bypassing

Roger Sayle <roger <at> eyesopen.com> writes:

> The following patch is my proposed fix for PR optimization/12628
...
> Unfortunately, calling reg_scan performs a traversal of the
> RTL insn stream, so we can expect a minor slow-down from this
> change :<.  It isn't clear whether GCSE or loop preserve the
> "reg_scan" information, so this change is safe even when the
> user hasn't specified wierd command line options.  However,
> to try and combat this, the patch below contains some clean-ups
> to reg_scan and reg_scan_update.  I've also moved reg_scan's
> call to allocate_reg_info (which looks like it can do a
> significant amount of work) into the TV_REG_SCAN timevar so
> we can get a more accurate picture of how much reg_scan costs.
> 
> 
> My next patch addresses PR/12322 and will speed-up RTL's GCSE
> pass, so hopefully we'll still have a net performance win.
> 
> 
> This patch has been tested on i686-pc-linux-gnu with a complete
> "make bootstrap", all languages except treelang, and regression
> tested with a top-level "make -k check" with no new failures.
> 
> Ok for mainline?

This is OK.  Thanks for the clear explanation of the patch, it helped
a lot.

I expect that your clean-ups have no effect on reg_scan's performance,

(Continue reading)