On Tue, Apr 2, 2013 at 5:00 AM, Jeff Law wrote:
> On 04/01/2013 01:47 PM, Eric Botcazou wrote:
>>>
>>> I'm not sure what this is supposed to do. How is pat == target ever
>>> going to be true when ANY_RETURN_P (pat) is true? Isn't target supposed
>>> to be a CODE_LABEL or NULL? What am I missing?
>>
>>
>> The simple_return change from Bernd. The JUMP_LABEL of a JUMP_INSN is now
>> either a CODE_LABEL or a RETURN or a SIMPLE_RETURN.
>
> Ah, OK, yea, it makes perfect sense now. Approved.
>
> If you wanted to get ambitious, rewriting reorg.c to compute and use
> dependency links to drive its candidate selection instead of the insane
> tracking code it uses would be a huge step forward, both in terms of
> cleanliness. It'd also help compile-time performance; we do a lot of work
> to track resources that would be totally unnecessary.
>
> I don't work on targets with delay slots anymore, so it's not something I'm
> likely to ever tackle myself.
For the last few, my "new development cycle resolutions" list has
included an item "do something about reorg.c" but every time the task
is just too daunting.
The following problems (at least these anyway) make it a difficult job:
* There is no CFG, and sched-deps.c doesn't workout one. Extending the
life of the CFG at least up to just before dbr_schedule() is a
chicken-and-egg problem. Some machine_reorg passes wreck the CFG
beyond repair. The MIPS back end has to run dbr_schedule() as part of
its machine_reorg pass before mips_reorg_process_insns. The SPARC back
end also calls dbr_schedule() in its machine_reorg pass to work around
errata in Atmel AT697F chips (LEON2-like, i.e. fairly new, see
r179921).
Ironically, there are also a few machine_reorgs *need* the CFG and
restore it just after pass_free_cfg...
* Even if there'd be a CFG, sched-deps.c doesn't handle sequences,
which may appear after the MIPS and SPARC machine_reorg passes, which
pass the delayed-branch scheduled insn stream through the rest of the
compiler passes pipeline, also through pass_delay_slots.
* sched-deps uses the DF machinery (DF_INSN_USES, DF_INSN_DEFS) but DF
doesn't look through SEQUENCEs so DF caches are invalid after, and
probably during, dbr_schedule().
* Generally the reorg.c code doesn't look inviting to someone willing
to spend some time editing it. A few functions are huge
(fill_slots_from_thread and relax_delay_slots ~500 each, 25% of
reorg.c). There are many recurring idioms that make the code somewhat
harder to read than necessary (e.g. NONJUMP_INSN_P && GET_CODE ==
SEQUENCE). And some conditionals look too complex to touch and apply
transformations in the middle of a condition, e.g. try_split in this
one:
/* If there are slots left to fill and our search was stopped by an
unconditional branch, try the insn at the branch target. We can
redirect the branch if it works.
Don't do this if the insn at the branch target is a branch. */
if (slots_to_fill != slots_filled
&& trial
&& jump_to_label_p (trial)
&& simplejump_p (trial)
&& (target == 0 || JUMP_LABEL (trial) == target)
&& (next_trial = next_active_insn (JUMP_LABEL (trial))) != 0
&& ! (NONJUMP_INSN_P (next_trial)
&& GET_CODE (PATTERN (next_trial)) == SEQUENCE)
&& !JUMP_P (next_trial)
&& ! insn_references_resource_p (next_trial, &set, true)
&& ! insn_sets_resource_p (next_trial, &set, true)
&& ! insn_sets_resource_p (next_trial, &needed, true)
#ifdef HAVE_cc0
&& ! reg_mentioned_p (cc0_rtx, PATTERN (next_trial))
#endif
&& ! (maybe_never && may_trap_or_fault_p (PATTERN (next_trial)))
&& (next_trial = try_split (PATTERN (next_trial), next_trial, 0))
&& eligible_for_delay (insn, slots_filled, next_trial, flags)
&& ! can_throw_internal (trial))
{
* reorg.c is doing things itself that it should be using existing APIs
for, e.g. emit_delay_sequence should use
add_insn_before/add_insn_after instead of splicing seq_insn into the
chain "manually".
If only there'd be a better way to model delayed insn (better than
SEQUENCE) then it is probably easier to pick Muchnick off the shelf
and re-implement delayed-branch scheduling from scratch, rather than
"modernizing" reorg.c.
Ciao!
Steven
