On Tue, Sep 18, 2012 at 1:31 PM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> This patch aims to fix all stability issues related to using the first >> scheduler in gcc >> for x86 target (there several reported issues related to this problem). >> >> Main idea of this activity is mostly to provide user a possibility to >> safely turn on first scheduler for his codes. In some cases this could >> positively affect performance, especially for in-order Atom. >> >> Below is short description of proposed changes. > >> 2012-09-18 Yuri Rumyantsev <ysrum...@gmail.com> >> >> * config/i386/i386.c (ix86_dep_by_shift_count_body) : Add >> check on reload_completed since it can be invoked before >> register allocation phase in 1st scheduler. >> (ia32_multipass_dfa_lookahead) : Do not use dfa_lookahead for 1st >> Scheduler to save compile time. >> (ix86_sched_reorder) : Do not perform ready list reordering for 1st >> Scheduler to save compile time. >> (insn_is_function_arg) : New function. Returns true if lhs of insn is >> HW function argument register. >> (add_parameter_dependencies) : New function. Add output dependencies >> for chain of function adjacent arguments if only there is a move to >> likely spilled HW registers. Return first argument if at least one >> dependence was added or NULL otherwise. >> (avoid_func_arg_motion) : New function. Add output or anti dependency >> from insn to first_arg to restrict code motion. >> (add_dependee_for_func_arg) : New function. Avoid cross block motion >> of >> function argument through adding dependency from the first non-jump >> insn in bb. >> (ix86_dependencies_evaluation_hook) : New function. Hook for >> schedule1: >> avoid motion of function arguments passed in passed in likely spilled >> HW registers. >> (ix86_adjust_priority) : New function. Hook for schedule1: set >> priority >> of moves from likely spilled HW registers to maximum to schedule them >> as soon as possible. >> (ix86_sched_init_global): Do not perform multipass scheduling for 1st >> Scheduler to save compile time. > > I would kindly ask scheduler expert to review the patch from the > scheduler functionality POV. I have received opinion from Vladimir from off-line discussion, quoted below: --quote-- I think, it is ok. Switching off first cycle multipass scheduling is ok. It is mostly useful when the order of insns issued on the same cycle is important (mostly VLIW or quasy-VLIW processors). Other solutions are necessary to decrease spills and avoid reload crash (can not find a register in a class) when the 1st insn scheduling is on. I don't think it fully avoids possibility of the reload crashes but it takes into account most of cases resulting in the crashes and makes the crash possibility really negligible. Register pressure sensitive insn scheduling decreased the possibility. This patch will make it negligible. And LRA will solve all the rest cases of the crashes. I don't like a bit absence in freedom of moving argument insns with likely spilled hard-regs between each other as they are chained in the original order but it is debatable because it still decreases the possibility of spills. In overall, the patch is ok for me. --/quote-- Based on this opinion, the patch is OK for mainline, if there are no objections from other x86 maintainers in the next couple of days (48h). However, please watch for possible fallout from the patch, compile-time ICEs and performance problems. x86 and scheduler didn't play well together in the past, but your patch and (in the near future) LRA seems to fix all these problems. Thanks, Uros.
