[PATCH] gcse.c: don't compute a mapping from CUID to INSN

[PATCH] gcse.c: don't compute a mapping from CUID to INSN

[Steven Bosscher]

Hi,

I've long lost the illusion that anyone here cares one serious bit
about compile time. But having loops over all insns just for the sake
of it is surely too much even for the most stubborn pass-adding,
compile time increasing gcc hacker around here?

Thus. Don't do useless work in gcse.c.  This saves 2 passes over all
insns with -fgcse, and even 3 with -Os.  I doubt this will have any
measurable impact, but one has to start somewhere.

Patch is untested beyond "it compiles", but that's really sufficient
in this case anyway :-)

Gr.
Steven

	* gcse.c (CUID_INSN): Remove.
	(cuid_insn): Ditto.
	(alloc_gcse_mem): Don't allocate cuid_insn.
	(free_gcse_mem): Don't free cuid_insn.

Re: [PATCH] gcse.c: don't compute a mapping from CUID to INSN

[Steven Bosscher]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

And with patch this time.

On Nov 9, 2007 11:46 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> Hi,
>
> I've long lost the illusion that anyone here cares one serious bit
> about compile time. But having loops over all insns just for the sake
> of it is surely too much even for the most stubborn pass-adding,
> compile time increasing gcc hacker around here?
>
> Thus. Don't do useless work in gcse.c.  This saves 2 passes over all
> insns with -fgcse, and even 3 with -Os.  I doubt this will have any
> measurable impact, but one has to start somewhere.
>
> Patch is untested beyond "it compiles", but that's really sufficient
> in this case anyway :-)
>
> Gr.
> Steven
>
>         * gcse.c (CUID_INSN): Remove.
>         (cuid_insn): Ditto.
>         (alloc_gcse_mem): Don't allocate cuid_insn.
>         (free_gcse_mem): Don't free cuid_insn.
>

[-- Attachment #2: gcse_no_cuid_to_insn_mapping.diff.txt --]
[-- Type: text/plain, Size: 1069 bytes --]

Index: gcse.c
===================================================================
--- gcse.c	(revision 130055)
+++ gcse.c	(working copy)
@@ -380,12 +380,6 @@
 /* Number of cuids.  */
 static int max_cuid;
 
-/* Mapping of cuids to insns.  */
-static rtx *cuid_insn;
-
-/* Get insn from cuid.  */
-#define CUID_INSN(CUID) (cuid_insn[CUID])
-
 /* Maximum register number in function prior to doing gcse + 1.
    Registers created during this pass have regno >= max_gcse_regno.
    This is named with "gcse" to not collide with global of same name.  */
@@ -941,16 +935,7 @@
 	else
 	  uid_cuid[INSN_UID (insn)] = i;
       }
-
-  /* Create a table mapping cuids to insns.  */
-
   max_cuid = i;
-  cuid_insn = gcalloc (max_cuid + 1, sizeof (rtx));
-  i = 0;
-  FOR_EACH_BB (bb)
-    FOR_BB_INSNS (bb, insn)
-      if (INSN_P (insn))
-	CUID_INSN (i++) = insn;
 
   /* Allocate vars to track sets of regs.  */
   reg_set_bitmap = BITMAP_ALLOC (NULL);
@@ -971,7 +956,6 @@
 free_gcse_mem (void)
 {
   free (uid_cuid);
-  free (cuid_insn);
 
   BITMAP_FREE (reg_set_bitmap);
 

Re: [PATCH] gcse.c: don't compute a mapping from CUID to INSN

[Paolo Bonzini]

Steven Bosscher wrote:
> And with patch this time.
> 
> On Nov 9, 2007 11:46 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> Hi,
>>
>> I've long lost the illusion that anyone here cares one serious bit
>> about compile time. But having loops over all insns just for the sake
>> of it is surely too much even for the most stubborn pass-adding,
>> compile time increasing gcc hacker around here?

It's actually possible to replace CUIDs with LUIDs altogether (the 
difference is only that LUIDs are per-basic-block, but this should not 
matter in any sane^Wmodern pass, and in fact for GCSE it does not).  I 
have a patch, but it's not suitable for stage3 (unless we use the excuse 
that df slowed down the compiler, and the patch exploits one benefit of df).

Paolo

Re: [PATCH] gcse.c: don't compute a mapping from CUID to INSN

[Steven Bosscher]

On Nov 11, 2007 9:46 AM, Paolo Bonzini <bonzini@gnu.org> wrote:

> It's actually possible to replace CUIDs with LUIDs altogether (the
> difference is only that LUIDs are per-basic-block, but this should not
> matter in any sane^Wmodern pass, and in fact for GCSE it does not).  I
> have a patch, but it's not suitable for stage3 (unless we use the excuse
> that df slowed down the compiler, and the patch exploits one benefit of df).

I have a patch just like that -- while working on it I found
CUID_INSN.  It seems to me that this would be a suitable change for
stage3, but you have to be careful because you will need new
df_analyze calls to update the LUIDs between the different passes.
Gr.
Steveb

Re: [PATCH] gcse.c: don't compute a mapping from CUID to INSN

[Richard Guenther]

On Nov 9, 2007 11:46 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> Hi,
>
> I've long lost the illusion that anyone here cares one serious bit
> about compile time. But having loops over all insns just for the sake
> of it is surely too much even for the most stubborn pass-adding,
> compile time increasing gcc hacker around here?
>
> Thus. Don't do useless work in gcse.c.  This saves 2 passes over all
> insns with -fgcse, and even 3 with -Os.  I doubt this will have any
> measurable impact, but one has to start somewhere.
>
> Patch is untested beyond "it compiles", but that's really sufficient
> in this case anyway :-)

If that works, it looks obvious indeed ;)

Ok for mainline.

Thanks,
Richard.

> Gr.
> Steven
>
>         * gcse.c (CUID_INSN): Remove.
>         (cuid_insn): Ditto.
>         (alloc_gcse_mem): Don't allocate cuid_insn.
>         (free_gcse_mem): Don't free cuid_insn.
>

Re: [PATCH] gcse.c: don't compute a mapping from CUID to INSN

[Jakub Jelinek]

On Fri, Nov 09, 2007 at 11:46:13PM +0100, Steven Bosscher wrote:
> Thus. Don't do useless work in gcse.c.  This saves 2 passes over all
> insns with -fgcse, and even 3 with -Os.  I doubt this will have any
> measurable impact, but one has to start somewhere.
> 
> Patch is untested beyond "it compiles", but that's really sufficient
> in this case anyway :-)
> 
> 	* gcse.c (CUID_INSN): Remove.
> 	(cuid_insn): Ditto.
> 	(alloc_gcse_mem): Don't allocate cuid_insn.
> 	(free_gcse_mem): Don't free cuid_insn.

I'd say just commit it as obvious, the last and only user of that has been
removed more than 3 years ago, incidentally by you:

2004-05-14  Steven Bosscher  <stevenb@suse.de>

        * passes.c (rest_of_handle_null_pointer): Remove.
        (rest_of_handle_cse): Don't call rest_of_handle_null_pointer.
        (rest_of_compilation): Likewise.
        * rtl.h (delete_null_pointer_checks): Remove prototype.
        * gcse.c (rd_kill, rd_gen, reaching_defs, rd_out, ae_in, ae_out):
        Remove declarations.
        (get_bitmap_width, alloc_rd_mem, free_rd_mem, handle_rd_kill_set,
        compute_kill_rd, compute_rd, alloc_avail_expr_mem,
        free_avail_expr_mem, compute_ae_gen, expr_killed_p, compute_ae_kill,
        expr_reaches_here_p, computing_insn, def_reaches_here_p,
        can_disregard_other_sets, handle_avail_expr, classic_gcse,
        one_classic_gcse_pass, invalidate_nonnull_info,
        delete_null_pointer_checks_1, delete_null_pointer_checks,
        expr_reached_here_p_work): Remove.
        (gcse_main): Do not perform classic GCSE when optimizing for size.
        (alloc_pre_mem, free_pre_mem): Don't touch ae_in and ae_out, they
        are never used.

	Jakub