On 17/09/13 03:16, bin.cheng wrote:
>
>
>> -----Original Message-----
>> From: Richard Earnshaw
>> Sent: Thursday, September 12, 2013 11:24 PM
>> To: Bin Cheng
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH ARM]Extend thumb1_reorg to save more comparison
>> instructions
>>
>> On 18/04/13 06:34, Bin Cheng wrote:
>>
>> Sorry for the delay, I've been trying to get my head around this one.
>>
>>> thumb1_reorg-20130417.txt
>>>
>>>
>>> Index: gcc/config/arm/arm.c
>>>
>> ==========================================================
>> =========
>>> --- gcc/config/arm/arm.c (revision 197562)
>>> +++ gcc/config/arm/arm.c (working copy)
>>> @@ -14026,6 +14026,7 @@ thumb1_reorg (void)
>>> rtx set, dest, src;
>>> rtx pat, op0;
>>> rtx prev, insn = BB_END (bb);
>>> + bool insn_clobbered = false;
>>>
>>> while (insn != BB_HEAD (bb) && DEBUG_INSN_P (insn))
>>> insn = PREV_INSN (insn);
>>> @@ -14034,12 +14035,29 @@ thumb1_reorg (void)
>>> if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn)
>>> continue;
>>>
>>> - /* Find the first non-note insn before INSN in basic block BB.
> */
>>> + /* Get the register with which we are comparing. */
>>> + pat = PATTERN (insn);
>>> + op0 = XEXP (XEXP (SET_SRC (pat), 0), 0);
>>> +
>>> + /* Find the first flag setting insn before INSN in basic block
>>> + BB. */
>>> gcc_assert (insn != BB_HEAD (bb));
>>> - prev = PREV_INSN (insn);
>>> - while (prev != BB_HEAD (bb) && (NOTE_P (prev) || DEBUG_INSN_P
>> (prev)))
>>> - prev = PREV_INSN (prev);
>>> + for (prev = PREV_INSN (insn);
>>> + (!insn_clobbered
>>> + && prev != BB_HEAD (bb)
>>> + && (NOTE_P (prev)
>>> + || DEBUG_INSN_P (prev)
>>> + || (GET_CODE (prev) == SET
>>
>> This can't be right. prev is an insn of some form, so the test that it is
> a SET will
>> always fail.
>>
>> What you need to do here is to initialize 'set' to null before the loop
> and then
>> have something like
>>
>> || ((set = single_set (prev)) != NULL
>>
>>> + && get_attr_conds (prev) == CONDS_NOCOND)));
>>> + prev = PREV_INSN (prev))
>>> + {
>>> + if (reg_set_p (op0, prev))
>>> + insn_clobbered = true;
>>> + }
>>>
>>> + /* Skip if op0 is clobbered by insn other than prev. */
>>> + if (insn_clobbered)
>>> + continue;
>>> +
>>> set = single_set (prev);
>>
>> This now becomes redundant and ...
>>
>>> if (!set)
>>> continue;
>>
>> This will be based on the set you extracted above.
>>
>
> Hi Richard, here is the updated patch according to your comments. Tested on
> thumb1, please review.
>
OK.
R.
