Bug 23898 - basic block reordering excessively increases code size; get_uncond_jump_length pessimistic
Summary: basic block reordering excessively increases code size; get_uncond_jump_lengt...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-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: 16996
  Show dependency treegraph
 
Reported: 2005-09-15 17:43 UTC by Jorn Wolfgang Rennecke
Modified: 2005-09-22 21:37 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-09-15 19:44:01


Attachments
fix for SH (1.82 KB, patch)
2005-09-15 17:51 UTC, Jorn Wolfgang Rennecke
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jorn Wolfgang Rennecke 2005-09-15 17:43:42 UTC
copy_bb_p uses uncond_jump_length in order to gauge
when code increase will be negative or neglegible
(at -Os) or acceptable (at -O2); however,
uncond_jump_length has the wrong value.
According to the comment in reorder_basic_blocks, it
expects to get a minimal length for an unconditional
jump when it calls get_uncond_jump_length in order
to set uncond_jump_length.  However,it gets a maximal
length instead.
get_uncond_jump_length returns 5 for i386, and 24 for SH.
Comment 1 Jorn Wolfgang Rennecke 2005-09-15 17:51:28 UTC
Created attachment 9736 [details]
fix for SH

This patch set fixes the problem for the SH.
The patch to sh_output_mi_thunk should no longer be necessary in mainline,
AFAICR
Kaz has fixed the generic code to get the line number information initialized.
If we want to use this approach, a tm.texi patch needs to be added, and machine

specific patches for the other ports are needed to get benefit of this patch.

Another approach would be to make the gen* programs create a minumum insn
length function, and use that instead.
Comment 2 Andrew Pinski 2005-09-15 19:44:00 UTC
Confirmed, this seems like a good approach at least to me.
Comment 3 GCC Commits 2005-09-20 21:48:43 UTC
Subject: Bug 23898

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	amylaar@gcc.gnu.org	2005-09-20 21:48:37

Modified files:
	gcc            : ChangeLog bb-reorder.c final.c genattr.c 
	                 genattrtab.c output.h 

Log message:
	PR rtl-optimization/23898
	* output.h (get_attr_min_length): Declare.
	* final.c (get_attr_length_1): New function, broken out of:
	(get_attr_length).
	(get_attr_min_length): New function.
	* bb-reorder.c (copy_bb_p, get_uncond_jump_length): Use it.
	(duplicate_computed_gotos): Likewise.
	* genattr.c (insn_min_length): Generate declaration.
	* genattrtab.c (min_fn, min_attr_value): New functions.
	(make_length_attrs): Generate insn_min_length.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9993&r2=2.9994
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/bb-reorder.c.diff?cvsroot=gcc&r1=1.112&r2=1.113
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/final.c.diff?cvsroot=gcc&r1=1.360&r2=1.361
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/genattr.c.diff?cvsroot=gcc&r1=1.65&r2=1.66
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/genattrtab.c.diff?cvsroot=gcc&r1=1.162&r2=1.163
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/output.h.diff?cvsroot=gcc&r1=1.160&r2=1.161

Comment 4 Andrew Pinski 2005-09-20 21:59:44 UTC
Fixed in 4.1.0.
Comment 5 Jan-Benedict Glaw 2005-09-22 21:33:50 UTC
The patch that was imported two days ago seems to break architectures that don't have length defines in their MD files (eg. VAX). I haven't checked if there are other architectures affected, though...
Comment 6 Andrew Pinski 2005-09-22 21:37:27 UTC
(In reply to comment #5)
> The patch that was imported two days ago seems to break architectures that don't have length
> defines in their MD files (eg. VAX). I haven't checked if there are other architectures affected, 
> though...

And that is PR 23991 which is about to be fixed.