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.

Reply via email to