RFC : Patch to make auto-increment generation more aggressive.
Kenneth Zadeck
zadeck@naturalbridge.com
Wed Sep 12 17:14:00 GMT 2007
Ramana Radhakrishnan wrote:
> Hi Kenneth,
>
>
>> I do not really like the way this patch is structured. i appears to be
>> just a hack in front of all of the table driven parts. This will make
>> it difficult to maintain things in the future. I would much prefer to
>> see this incorporated into the table driven structure for the rest of
>> the pass.
>>
>
> Thanks for the feedback. I was originally trying to integrate it into
> the table driven structure, but wanted a quick prototype to check if
> the idea was worth it or not since I started work on this just a few
> days back. I'll work on integrating it into the table driven
> approach.
>
>
The idea is fine, but i do not want to approve the quick prototype.
> I did check this on a private port with post increment and post
> decrement and I saw a win in the performance for memset where I had a
> larger number of post increments being generated.
>
>
I think that people will want to see this work on something that they
can get their hands on before it goes in. I believe that you can get an
account on an ia-64 from
rick.jones2@hp.com.
Kenny
>
>> Furthermore, we are currently in stage III so unless prior arrangements
>> have been made with mark mitchell, or he approves it, the patch needs to
>> wait til stage I reopens.
>>
>
> I have not made prior arrangements yet but I have pinged Mark about it
> today. If it can be got in I'd like to get it in or will wait until
> stage1 is reopened.
>
> I will test it on the ARM too.
>
> cheers
> Ramana
>
> On 9/12/07, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
>
>> Ramana Radhakrishnan wrote:
>>
>>> Hi,
>>>
>>> We could improve auto-increment for ports that have post modify or
>>> post increment to generate better code with the attached patch.
>>>
>>> Currently we disable post_inc generation for the following case.
>>>
>>> *(a+c1)
>>> a = a + c2
>>>
>>> The attached patch transforms this (with the necessary checks) ofcourse to
>>> a = a + c1
>>> *(a+=(c2 - c1))
>>>
>>> provided ofcourse that the data flow requirements and other machine
>>> constraints are met. I am currently bootstrapping and regression
>>> testing this on x86-64-linux .
>>>
>>> It has some extra printf's but these will be cleared up before I
>>> submit it finally.
>>>
>>> cheers
>>> Ramana
>>>
>>>
>>>
>>>
>>>
>> I do not really like the way this patch is structured. i appears to be
>> just a hack in front of all of the table driven parts. This will make
>> it difficult to maintain things in the future. I would much prefer to
>> see this incorporated into the table driven structure for the rest of
>> the pass.
>>
>> Furthermore, we are currently in stage III so unless prior arrangements
>> have been made with mark mitchell, or he approves it, the patch needs to
>> wait til stage I reopens.
>>
>> Also this patch needs to be tested on machines that have post increment
>> and decrement.
>> this pass is a noop on the x86-64 since it has no post increment
>> instructions.
>> I know that the ia-64 has post inc/dec. I believe that the arm and the
>> m68k also do but i am not sure.
>>
>> Kenny
>>
>>
>>> ------------------------------------------------------------------------
>>>
>>> Index: auto-inc-dec.c
>>> ===================================================================
>>> RCS file: /srv/cvs/icera_gnu_tools/gnu-4.3/gcc/auto-inc-dec.c,v
>>> retrieving revision 1.1.1.1.2.1
>>> diff -a -u -r1.1.1.1.2.1 auto-inc-dec.c
>>> --- auto-inc-dec.c 20 Jul 2007 12:33:08 -0000 1.1.1.1.2.1
>>> +++ auto-inc-dec.c 12 Sep 2007 06:56:14 -0000
>>> @@ -149,6 +149,9 @@
>>> becomes
>>>
>>> *(a += c) pre
>>> +
>>> +
>>> +
>>> */
>>> #ifdef AUTO_INC_DEC
>>>
>>> @@ -692,12 +695,64 @@
>>> [inc_insn.reg1_state][mem_insn.reg1_state][inc_insn.form];
>>>
>>> if (dbg_cnt (auto_inc_dec) == false)
>>> - return false;
>>> + return false;
>>> +
>>> + if (inc_insn.form == FORM_POST_INC &&
>>> + mem_insn.reg1_is_const &&
>>> + inc_insn.reg1_is_const &&
>>> + (mem_insn.reg1_val != inc_insn.reg1_val)
>>> + && (mem_insn.reg1_val != -inc_insn.reg1_val))
>>> + {
>>> + /* This is the form where you have :
>>> + * mem_insn of the form
>>> + * * ( a + const1) = something
>>> + * a = a + const2
>>> + * Transform this to
>>> + * a = a + const1
>>> + * * ( a += const2 - const1)
>>> + */
>>> + if (attempt_change (gen_rtx_POST_MODIFY (Pmode,
>>> + inc_reg,
>>> + plus_constant(inc_reg, inc_insn.reg1_val -
>>> + mem_insn.reg1_val)),
>>> + inc_reg))
>>> + {
>>> + rtx insn;
>>> + int regno;
>>> + if (dump_file)
>>> + fprintf (dump_file, "Attempting to change over\n");
>>> + /* We still have to emit an additional add before the mem insn
>>> + and delete the inc insn. The lower inc insn is deleted
>>> + in attempt_change already. Here we just set up the
>>> + additional add instruction.
>>> + */
>>> +
>>> + // insn = emit_insn_before (gen_rtx_SET ( Pmode, inc_reg, gen_rtx_PLUS ( Pmode, inc_reg,
>>> + // mem_insn.reg1)), mem_insn.insn);
>>> +
>>> +
>>>
>>>
>> do not use this kind of comment.
>>
>>
>>> + insn = emit_insn_before (gen_rtx_SET ( Pmode, inc_reg, plus_constant(inc_reg,
>>> + mem_insn.reg1_val)), mem_insn.insn);
>>> + regno = REGNO (inc_reg);
>>> + reg_next_def[regno] = insn;
>>> + reg_next_use[regno] = mem_insn.insn;
>>> + reg_next_inc_use[regno] = insn;
>>> + df_recompute_luids (BASIC_BLOCK (BLOCK_NUM (mem_insn.insn)));
>>> +
>>> + return true;
>>> + }
>>> + else
>>> + {
>>> + return false;
>>> + }
>>> + }
>>>
>>> switch (gen_form)
>>> {
>>> default:
>>> case NOTHING:
>>> + if (dump_file)
>>> + fprintf (dump_file, "try_merge gen_form is nothing\n");
>>> return false;
>>>
>>> case SIMPLE_PRE_INC: /* ++size */
>>> @@ -1016,7 +1071,11 @@
>>> }
>>>
>>> if (dump_file)
>>> - dump_mem_insn (dump_file);
>>> + {
>>> + fprintf (dump_file, "From find_inc\n");
>>> + dump_mem_insn (dump_file);
>>> + fprintf (dump_file, "End of marker find inc1\n");
>>> + }
>>>
>>> /* Find the next use that is an inc. */
>>> insn = get_next_ref (REGNO (mem_insn.reg0),
>>> @@ -1069,8 +1128,12 @@
>>> }
>>>
>>> if (dump_file)
>>> - dump_inc_insn (dump_file);
>>> -
>>> + {
>>> + fprintf (dump_file, "Find inc marker dump_inc_insn #1\n");
>>> + dump_inc_insn (dump_file);
>>> + fprintf (dump_file, "End Find inc marker dump_inc_insn #1\n");
>>> + }
>>> +
>>> if (inc_insn.form == FORM_POST_ADD)
>>> {
>>> /* Make sure that there is no insn that assigns to inc_insn.res
>>> @@ -1151,9 +1214,14 @@
>>> }
>>> /* Both the inc/add and the mem have a constant. Need to check
>>> that the constants are ok. */
>>> - else if ((mem_insn.reg1_val != inc_insn.reg1_val)
>>> + else if ((inc_insn.form != FORM_POST_INC) &&
>>> + (mem_insn.reg1_val != inc_insn.reg1_val)
>>> && (mem_insn.reg1_val != -inc_insn.reg1_val))
>>> - return false;
>>> + {
>>> + if (dump_file)
>>> + fprintf (dump_file, "Returning false because constants are not equal\n");
>>> + return false;
>>> + }
>>> }
>>> else
>>> {
>>> @@ -1278,9 +1346,14 @@
>>> in this form, we need to make sure that there
>>> were no intervening uses of reg. */
>>> if (inc_insn.insn != other_insn)
>>> - return false;
>>> + {
>>> + if (dump_file)
>>> + fprintf (dump_file, "not the other insn\n");
>>> + return false;
>>> + }
>>> }
>>> -
>>> + if (dump_file)
>>> + fprintf (dump_file, " to try merge\n");
>>> return try_merge ();
>>> }
>>>
>>> @@ -1425,7 +1498,11 @@
>>> }
>>>
>>> if (dump_file)
>>> + {
>>> dump_inc_insn (dump_file);
>>> + if (dump_file)
>>> + fprintf (dump_file, "ok is %s\n", ok? "true":"false");
>>> + }
>>>
>>> if (ok && find_address (&PATTERN (mem_insn.insn)) == -1)
>>> {
>>>
>>>
>>
>
>
>
More information about the Gcc-patches
mailing list