On Tue, Nov 26, 2013 at 9:34 PM, Jeff Law <l...@redhat.com> wrote:
> On 11/26/13 12:33, Wei Mi wrote:
>>
>> On Mon, Nov 25, 2013 at 2:12 PM, Jeff Law <l...@redhat.com> wrote:
>>>
>>>
>>>>
>>>> Doing the cleanup at the end of BB could ensure all the groups
>>>> inserted for macrofusion will be cleaned. For groups not at the end of
>>>> a block, no matter whether they are cleaned up or not, nothing will
>>>> happen because other passes will not mess up those groups -- you said
>>>> cc0-setter/cc0-user was such a case. Is it call group a different
>>>> case?
>>>
>>>
>>> True, it would be safe, but it seems inconsistent and confusing that some
>>> SCHED_GROUP_P references would be purged and others remain.
>>>
>>> Given SCHED_GROUP_P is to used strictly in the scheduler ISTM that we
>>> should
>>> be wiping it as we leave and that our RTL checkers ought to be verifying
>>> there are no insns with SCHED_GROUP_P left on.
>>>
>>
>> How about add a verifier TODO_verify_sched_group_flag similar as
>> TODO_verify_rtl_sharing, and add the verifier in the todo lists of all
>> the scheduling passes.
>>
>>>
>>>>
>>>> For sched1 and sched2, we can do that. Actually, I find it has been
>>>> done in move_insn when commit_schedule. But for modulo scheduling, I
>>>> havn't found a good place to do it.
>>>
>>>
>>> Well, that's where I'd suggest focusing attention.
>>>
>>> jeff
>>>
>>
>> After looking it carefully, even for sched1 and sched2, it is not ok
>> to depend on move_insn in commit_schedule to clean up all the
>> SCHED_GROUP_P, suppose a block is decided not to be scheduled by
>> dbg_cnt, then SCHED_GROUP_P inside the block will not be cleaned.
>> It is even more difficult to find a place inside SMS scheduling to do
>> the cleanup.
>>
>> Any suggestions?
>
> Hmm, maybe attack from the other direction? -- could we clear SCHED_GROUP_P
> for each insn at the start of this loop in sched_analyze?
>
> It's not as clean in the sense that SCHED_GROUP_P "escapes" the scheduler,
> but it might be an option.
>
> for (insn = head;; insn = NEXT_INSN (insn))
> {
>
> if (INSN_P (insn))
> {
> /* And initialize deps_lists. */
> sd_init_insn (insn);
> }
>
> deps_analyze_insn (deps, insn);
>
> if (insn == tail)
> {
> if (sched_deps_info->use_cselib)
> cselib_finish ();
> return;
> }
> }
> Jeff
>>
>>
>
Thanks for the suggestion. It looks workable. Then I need to move the
SCHED_GROUP_P setting for macrofusion from sched_init to a place
inside sched_analyze after the SCHED_GROUP_P cleanup. It will be more
consistent with the settings for cc0 setter-user group and call group,
which are both inside sched_analyze.
I am trying this method...
Thanks,
Wei.
