* Re: Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
@ 2008-08-29 15:10 David Edelsohn
2008-08-31 13:35 ` Andrey Belevantsev
0 siblings, 1 reply; 8+ messages in thread
From: David Edelsohn @ 2008-08-29 15:10 UTC (permalink / raw)
To: Andrey Belevantsev
Cc: GCC Patches, Jim Wilson, Vladimir Makarov, Ayal Zaks, Mark Mitchell
* config/rs6000/rs6000.c (rs6000_init_sched_context,
rs6000_alloc_sched_context, rs6000_set_sched_context,
rs6000_free_sched_context): New functions.
(struct _rs6000_sched_context): New.
(rs6000_sched_reorder2): Do not modify INSN_PRIORITY for selective
scheduling.
(rs6000_sched_finish): Do not run for selective scheduling.
The rs6000 part of the patch is okay with a modification to the following chunk:
*************** rs6000_sched_finish (FILE *dump, int sch
*** 20085,20091 ****
if (reload_completed && rs6000_sched_groups)
{
! if (rs6000_sched_insert_nops == sched_finish_none)
return;
if (rs6000_sched_insert_nops == sched_finish_pad_groups)
--- 20103,20110 ----
if (reload_completed && rs6000_sched_groups)
{
! if (rs6000_sched_insert_nops == sched_finish_none
! || sel_sched_p ())
return;
if (rs6000_sched_insert_nops == sched_finish_pad_groups)
Please change this to a separate test for clarify
+ /* Do not run sched_finish hook when selective scheduling enabled. */
+ if (sel_sched_p ())
+ return;
+
if (rs6000_sched_insert_nops == sched_finish_none)
return;
instead of combining the tests.
Also, target maintainers have flexibility during stage 3 with respect
to changes local to a port,
so the Itanium changes can be approved and committed during stage 3,
although earlier would
be better.
Thanks, David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
2008-08-29 15:10 Selective scheduling pass - target changes (ia64 & rs6000) [3/3] David Edelsohn
@ 2008-08-31 13:35 ` Andrey Belevantsev
0 siblings, 0 replies; 8+ messages in thread
From: Andrey Belevantsev @ 2008-08-31 13:35 UTC (permalink / raw)
To: David Edelsohn
Cc: GCC Patches, Jim Wilson, Vladimir Makarov, Ayal Zaks, Mark Mitchell
Hello,
David Edelsohn wrote:
> * config/rs6000/rs6000.c (rs6000_init_sched_context,
> rs6000_alloc_sched_context, rs6000_set_sched_context,
> rs6000_free_sched_context): New functions.
> (struct _rs6000_sched_context): New.
> (rs6000_sched_reorder2): Do not modify INSN_PRIORITY for selective
> scheduling.
> (rs6000_sched_finish): Do not run for selective scheduling.
>
> The rs6000 part of the patch is okay with a modification to the following chunk:
Thanks for the review, I'll fix that up.
> Also, target maintainers have flexibility during stage 3 with respect
> to changes local to a port,
> so the Itanium changes can be approved and committed during stage 3,
> although earlier would
> be better.
I didn't know that. But, what would be the preferred policy for
checking in the scheduler in this case? Should I wait for the Itanium
changes to be reviewed and then commit the whole patch, which means that
target-independent changes would be committed during stage3? Or, should
I check in the scheduler without ia64 changes now, which means it will
be non-functional on Itanium, and commit to reverting it in case the
ia64 changes will not be reviewed even during stage 3? I'll appreciate
the advice on how to proceed.
Thanks, Andrey
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC] Selective scheduling pass
@ 2008-06-03 14:24 Andrey Belevantsev
2008-06-03 14:28 ` Selective scheduling pass - target changes (ia64 & rs6000) [3/3] Andrey Belevantsev
0 siblings, 1 reply; 8+ messages in thread
From: Andrey Belevantsev @ 2008-06-03 14:24 UTC (permalink / raw)
To: GCC Patches; +Cc: Jim Wilson, Vladimir Makarov
Hello,
The patches in this thread introduce selective scheduler in GCC,
implemented by myself, Dmitry Melnik, Dmitry Zhurikhin, Alexander
Monakov, and Maxim Kuvyrkov while he was at ISP RAS. Selective
scheduler is aimed at scheduling eager targets such as ia64, power6, and
cell. The implementation contains both the scheduler and the software
pipeliner, which can be used on loops with control flow not handled by
SMS. The scheduler can work either before or after register allocation,
but it is currently tuned to work after.
The scheduler was bootstrapped and tested on ia64, with all default
languages, both as a first and as a second scheduler. It was also
bootstrapped with c, c++, and fortran enabled on ppc64 and x86-64.
On ia64, test results on SPEC2k FP comparing -O3 -ffast-math on trunk
and sel-sched branch show 3.8% speedup on average, SPEC INT shows both
small speedups and regressions, staying around neutral in average:
168.wupwise 513 552 7,60%
171.swim 757 772 1,98%
172.mgrid 570 643 12,81%
173.applu 503 524 4,17%
177.mesa 796 795 -0,13%
178.galgel 814 787 -3,32%
179.art 1990 2098 5,43%
183.equake 513 569 10,92%
187.facerec 958 991 3,44%
188.ammp 765 775 1,31%
189.lucas 860 869 1,05%
191.fma3d 549 536 -2,37%
200.sixtrack 300 323 7,67%
301.apsi 522 546 4,60%
Geomean 673,97 699,87 3,84%
164.gzip 683 682 -0,15%
175.vpr 814 802 -1,47%
176.gcc 1080 1069 -1,02%
181.mcf 701 708 1,00%
186.crafty 872 855 -1,95%
197.parser 729 728 -0,14%
252.eon 793 785 -1,01%
253.perlbmk 824 839 1,82%
254.gap 558 569 1,97%
255.vortex 1012 966 -4,55%
256.bzip2 758 762 0,53%
300.twolf 1005 1015 1,00%
Geomean 806,04 803,25 -0,35%
On power6, Revital Eres saw speedups on several tests; additional tuning
is required to get good results there, which is complicated because we
don't have power6. On cell, there was some third-party testing in 2007,
showing 4-6% speedups, but I don't have more detailed information.
Compile time slowdown measured with --enable-checking=assert is quite
significant -- about 12% on spec int and about 18% on spec fp and
cc1-i-files collection. For this reason, we have enabled selective
scheduler by default at -O3 on ia64 and disabled by default on other
targets.
Our current plan is to work on further compile time improvements and
performance tuning for ppc and cell, hopefully with the help of IBM
Haifa folks. If we will complete this work before the end of stage2,
then we can enable selective scheduling at -O3 also for ppc in 4.4. In
the mid-term, we will work on removing the ebb scheduler, as it is now
used on ia64 only and will be superseded by selective scheduler when
we'll further improve compile time.
Andrey
^ permalink raw reply [flat|nested] 8+ messages in thread
* Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
2008-06-03 14:24 [RFC] Selective scheduling pass Andrey Belevantsev
@ 2008-06-03 14:28 ` Andrey Belevantsev
2008-08-22 16:04 ` Andrey Belevantsev
0 siblings, 1 reply; 8+ messages in thread
From: Andrey Belevantsev @ 2008-06-03 14:28 UTC (permalink / raw)
To: GCC Patches; +Cc: Jim Wilson, Vladimir Makarov, Ayal Zaks
[-- Attachment #1: Type: text/plain, Size: 8286 bytes --]
Hello,
This patch shows the target dependent changes for the selective
scheduler. The majority of changes is in the config/ia64/ia64.c file.
They also include a lot of tunings done thorough the project. Each
tuning originated from a problem test case (usually from SPEC or from Al
Aburto tests) that was fixed by it. The summary of changes is as follows:
o speculation support is improved to allow more patterns to be
speculative (speculable1 and speculable2 attributes mark
patterns/alternatives that are valid for speculation);
o bundling also optimizes for minimal number of mid-bundle stops;
o we lower the priority of memory operations if we have issued too many
of them on the current cycle;
o default function and loop alignment is set to 64 and 32, respectively;
o we discard cost of memory dependencies which are likely false;
o we place a stop bit after every simulated processor cycle;
o the incorrect bypass in itanium2.md that resulted in stalls between
fma and st insns is removed.
Also, to support the proper alignment on scheduled loops, we have put
pass_compute_alignments after pass_machine_reorg (this part actually is
in the middle-end patch, but I mention it here as it was inspired by the
Itanium).
The rs6000 change is a minimal version needed to support the selective
scheduler for a target. As we now can have several points in a region
at which we are scheduling, the backend can no longer save the scheduler
state in private variables and use it in the hooks (e.g.
last_scheduled_insn). For that purpose, a concept of a target context
is introduced: all private scheduler-related target info should be put
in there, and the target should provide hooks for
creating/deleting/setting as current a target context. The scheduler
then treats target contexts as opaque pointers. Also, we do not yet
support adjust_priority hooks (but the work on this is underway), so
that part of the rs6000 scheduler hooks is disabled.
OK for trunk?
Andrey
2008-06-03 Andrey Belevantsev <abel@ispras.ru>
Dmitry Melnik <dm@ispras.ru>
Dmitry Zhurikhin <zhur@ispras.ru>
Alexander Monakov <amonakov@ispras.ru>
Maxim Kuvyrkov <maxim@codesourcery.com>
* config/ia64/ia64.c: Include sel-sched.h. Rewrite speculation hooks.
(ia64_gen_spec_insn): Removed.
(get_spec_check_gen_function, insn_can_be_in_speculative_p,
ia64_gen_spec_check): New static functions.
(ia64_alloc_sched_context, ia64_init_sched_context,
ia64_set_sched_context, ia64_clear_sched_context,
ia64_free_sched_context, ia64_get_insn_spec_ds,
ia64_get_insn_checked_ds, ia64_skip_rtx_p): Declare functions.
(ia64_needs_block_p): Change prototype.
(ia64_gen_check): Rename to ia64_gen_spec_check.
(ia64_adjust_cost): Rename to ia64_adjust_cost_2. Add new parameter
into declaration, add special memory dependencies handling.
(TARGET_SCHED_ALLOC_SCHED_CONTEXT, TARGET_SCHED_INIT_SCHED_CONTEXT,
TARGET_SCHED_SET_SCHED_CONTEXT, TARGET_SCHED_CLEAR_SCHED_CONTEXT,
TARGET_SCHED_FREE_SCHED_CONTEXT, TARGET_SCHED_GET_INSN_SPEC_DS,
TARGET_SCHED_GET_INSN_CHECKED_DS, TARGET_SCHED_SKIP_RTX_P):
Define new target hooks.
(TARGET_SCHED_GEN_CHECK): Rename to TARGET_SCHED_GEN_SPEC_CHECK.
(ia64_override_options): Turn on selective scheduling with -O3,
disable -fauto-inc-dec. Initialize align_loops and align_functions
to 32 and 64, respectively. Set global selective scheduling flags
according to target-dependent flags.
(rtx_needs_barrier): Support UNSPEC_LDS_A.
(group_barrier_needed): Use new mstop-bit-before-check flag.
Add heuristic.
(dfa_state_size): Make global.
(spec_check_no, max_uid): Remove.
(mem_ops_in_group, current_cycle): New variables.
(ia64_sched_init): Disable checks for !SCHED_GROUP_P after reload.
Initialize new variables.
(is_load_p, record_memory_reference): New functions.
(ia64_dfa_sched_reorder): Lower priority of loads when limit is
reached.
(ia64_variable_issue): Change use of current_sched_info to
sched_deps_info. Update comment. Note if a load or a store is issued.
(ia64_first_cycle_multipass_dfa_lookahead_guard_spec): Require
a cycle
advance if maximal number of loads or stores was issued on current
cycle.
(scheduled_good_insn): New static helper function.
(ia64_dfa_new_cycle): Assert that last_scheduled_insn is set when
a group barrier is needed. Fix vertical spacing. Guard the code
doing state transition with last_scheduled_insn check.
Mark that a stop bit should be before current insn if there was a
cycle advance. Update current_cycle and mem_ops_in_group.
(ia64_h_i_d_extended): Change use of current_sched_info to
sched_deps_info. Reallocate stops_p by larger chunks.
(struct _ia64_sched_context): New structure.
(ia64_sched_context_t): New typedef.
(ia64_alloc_sched_context, ia64_init_sched_context,
ia64_set_sched_context, ia64_clear_sched_context,
ia64_free_sched_context): New static functions.
(gen_func_t): New typedef.
(get_spec_load_gen_function): New function.
(SPEC_GEN_EXTEND_OFFSET): Declare.
(ia64_set_sched_flags): Check common_sched_info instead of *flags.
(get_mode_no_for_insn): Change the condition that prevents use of
special hardware registers so it can now handle pseudos.
(get_spec_unspec_code): New function.
(ia64_skip_rtx_p, get_insn_spec_code, ia64_get_insn_spec_ds,
ia64_get_insn_checked_ds, ia64_gen_spec_load): New static functions.
(ia64_speculate_insn, ia64_needs_block_p): Support branchy checks
during selective scheduling.
(ia64_speculate_insn): Use ds_get_speculation_types when
determining whether we need to change the pattern.
(SPEC_GEN_LD_MAP, SPEC_GEN_CHECK_OFFSET): Declare.
(ia64_spec_check_src_p): Support new speculation/check codes.
(struct bundle_state): New field.
(issue_nops_and_insn): Initialize it.
(insert_bundle_state): Minimize mid-bundle stop bits.
(important_for_bundling_p): New function.
(get_next_important_insn): Use important_for_bundling_p.
(bundling): When shifting TImode from unimportant insns, ignore
also group barriers. Assert that best state is found before
the backward bundling pass. Print number of mid-bundle stop bits.
Minimize mid-bundle stop bits. Check correct calculation of
mid-bundle stop bits.
(ia64_sched_finish, final_emit_insn_group_barriers): Fix formatting.
(final_emit_insn_group_barriers): Emit stop bits before insns starting
a new cycle.
(sel2_run): New variable.
(ia64_reorg): When flag_selective_scheduling is set, run the selective
scheduling pass instead of schedule_ebbs. Adjust for
flag_selective_scheduling2.
(ia64_optimization_options): Declare new parameter.
* config/ia64/ia64.md (speculable1, speculable2): New attributes.
(UNSPEC_LDS_A): New UNSPEC.
(movqi_internal, movhi_internal, movsi_internal, movdi_internal,
movti_internal, movsf_internal, movdf_internal,
movxf_internal): Make visible. Add speculable* attributes.
(output_c_nc): New mode attribute.
(mov<mode>_speculative_a, zero_extend<mode>di2_speculative_a,
mov<mode>_nc, zero_extend<mode>di2_nc,
advanced_load_check_nc_<mode>): New insns.
(zero_extend*): Add speculable* attributes.
* config/ia64/ia64.opt (msched_fp_mem_deps_zero_cost): New option.
(msched-stop-bits-after-every-cycle): Likewise.
(mstop-bit-before-check): Likewise.
(msched-max-memory-insns,
msched-max-memory-insns-hard-limit): Likewise.
(msched-spec-verbose, msched-prefer-non-data-spec-insns,
msched-prefer-non-control-spec-insns,
msched-count-spec-in-critical-path,
msel-sched-renaming, msel-sched-substitution,
msel-sched-data-spec,
msel-sched-control-spec, msel-sched-dont-check-control-spec):
Use Target
Report Var instead of Common Report Var.
* config/ia64/itanium2.md: Remove strange bypass.
* config/ia64/t-ia64 (ia64.o): Add dependency on sel-sched.h.
* config/rs6000/rs6000.c (rs6000_init_sched_context,
rs6000_alloc_sched_context, rs6000_set_sched_context,
rs6000_free_sched_context): New functions.
(struct _rs6000_sched_context): New.
(rs6000_sched_reorder2): Do not modify INSN_PRIORITY for selective
scheduling.
(rs6000_sched_finish): Do not run for selective scheduling.
[-- Attachment #2: sel-sched-merge-targets.diff.gz --]
[-- Type: application/gzip, Size: 21330 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
2008-06-03 14:28 ` Selective scheduling pass - target changes (ia64 & rs6000) [3/3] Andrey Belevantsev
@ 2008-08-22 16:04 ` Andrey Belevantsev
2008-09-25 22:39 ` sje
0 siblings, 1 reply; 8+ messages in thread
From: Andrey Belevantsev @ 2008-08-22 16:04 UTC (permalink / raw)
To: GCC Patches; +Cc: Jim Wilson, Vladimir Makarov, Ayal Zaks
[-- Attachment #1: Type: text/plain, Size: 6719 bytes --]
Hello,
This is the updated patch which I resend with the other parts. I think
the only change is that the flag_selective_scheduling option is turned
on in ia64_optimization_options, not in ia64_override_options.
This is actually the last part of the selective scheduler patch that did
not get reviewed yet. Maybe a global write maintainer and a rs6000
maintainer could have a look?
Thanks, Andrey
2008-08-22 Andrey Belevantsev <abel@ispras.ru>
Dmitry Melnik <dm@ispras.ru>
Dmitry Zhurikhin <zhur@ispras.ru>
Alexander Monakov <amonakov@ispras.ru>
Maxim Kuvyrkov <maxim@codesourcery.com>
* config/ia64/ia64.c: Include sel-sched.h. Rewrite speculation hooks.
(ia64_gen_spec_insn): Removed.
(get_spec_check_gen_function, insn_can_be_in_speculative_p,
ia64_gen_spec_check): New static functions.
(ia64_alloc_sched_context, ia64_init_sched_context,
ia64_set_sched_context, ia64_clear_sched_context,
ia64_free_sched_context, ia64_get_insn_spec_ds,
ia64_get_insn_checked_ds, ia64_skip_rtx_p): Declare functions.
(ia64_needs_block_p): Change prototype.
(ia64_gen_check): Rename to ia64_gen_spec_check.
(ia64_adjust_cost): Rename to ia64_adjust_cost_2. Add new parameter
into declaration, add special memory dependencies handling.
(TARGET_SCHED_ALLOC_SCHED_CONTEXT, TARGET_SCHED_INIT_SCHED_CONTEXT,
TARGET_SCHED_SET_SCHED_CONTEXT, TARGET_SCHED_CLEAR_SCHED_CONTEXT,
TARGET_SCHED_FREE_SCHED_CONTEXT, TARGET_SCHED_GET_INSN_SPEC_DS,
TARGET_SCHED_GET_INSN_CHECKED_DS, TARGET_SCHED_SKIP_RTX_P):
Define new target hooks.
(TARGET_SCHED_GEN_CHECK): Rename to TARGET_SCHED_GEN_SPEC_CHECK.
(ia64_optimization_options): Turn on selective scheduling with -O3,
disable -fauto-inc-dec.
(ia64_override_options): Initialize align_loops and align_functions
to 32 and 64, respectively. Set global selective scheduling flags
according to target-dependent flags.
(rtx_needs_barrier): Support UNSPEC_LDS_A.
(group_barrier_needed): Use new mstop-bit-before-check flag.
Add heuristic.
(dfa_state_size): Make global.
(spec_check_no, max_uid): Remove.
(mem_ops_in_group, current_cycle): New variables.
(ia64_sched_init): Disable checks for !SCHED_GROUP_P after reload.
Initialize new variables.
(is_load_p, record_memory_reference): New functions.
(ia64_dfa_sched_reorder): Lower priority of loads when limit is
reached.
(ia64_variable_issue): Change use of current_sched_info to
sched_deps_info. Update comment. Note if a load or a store is issued.
(ia64_first_cycle_multipass_dfa_lookahead_guard_spec): Require
a cycle
advance if maximal number of loads or stores was issued on current
cycle.
(scheduled_good_insn): New static helper function.
(ia64_dfa_new_cycle): Assert that last_scheduled_insn is set when
a group barrier is needed. Fix vertical spacing. Guard the code
doing state transition with last_scheduled_insn check.
Mark that a stop bit should be before current insn if there was a
cycle advance. Update current_cycle and mem_ops_in_group.
(ia64_h_i_d_extended): Change use of current_sched_info to
sched_deps_info. Reallocate stops_p by larger chunks.
(struct _ia64_sched_context): New structure.
(ia64_sched_context_t): New typedef.
(ia64_alloc_sched_context, ia64_init_sched_context,
ia64_set_sched_context, ia64_clear_sched_context,
ia64_free_sched_context): New static functions.
(gen_func_t): New typedef.
(get_spec_load_gen_function): New function.
(SPEC_GEN_EXTEND_OFFSET): Declare.
(ia64_set_sched_flags): Check common_sched_info instead of *flags.
(get_mode_no_for_insn): Change the condition that prevents use of
special hardware registers so it can now handle pseudos.
(get_spec_unspec_code): New function.
(ia64_skip_rtx_p, get_insn_spec_code, ia64_get_insn_spec_ds,
ia64_get_insn_checked_ds, ia64_gen_spec_load): New static functions.
(ia64_speculate_insn, ia64_needs_block_p): Support branchy checks
during selective scheduling.
(ia64_speculate_insn): Use ds_get_speculation_types when
determining whether we need to change the pattern.
(SPEC_GEN_LD_MAP, SPEC_GEN_CHECK_OFFSET): Declare.
(ia64_spec_check_src_p): Support new speculation/check codes.
(struct bundle_state): New field.
(issue_nops_and_insn): Initialize it.
(insert_bundle_state): Minimize mid-bundle stop bits.
(important_for_bundling_p): New function.
(get_next_important_insn): Use important_for_bundling_p.
(bundling): When shifting TImode from unimportant insns, ignore
also group barriers. Assert that best state is found before
the backward bundling pass. Print number of mid-bundle stop bits.
Minimize mid-bundle stop bits. Check correct calculation of
mid-bundle stop bits.
(ia64_sched_finish, final_emit_insn_group_barriers): Fix formatting.
(final_emit_insn_group_barriers): Emit stop bits before insns starting
a new cycle.
(sel2_run): New variable.
(ia64_reorg): When flag_selective_scheduling is set, run the selective
scheduling pass instead of schedule_ebbs. Adjust for
flag_selective_scheduling2.
(ia64_optimization_options): Declare new parameter.
* config/ia64/ia64.md (speculable1, speculable2): New attributes.
(UNSPEC_LDS_A): New UNSPEC.
(movqi_internal, movhi_internal, movsi_internal, movdi_internal,
movti_internal, movsf_internal, movdf_internal,
movxf_internal): Make visible. Add speculable* attributes.
(output_c_nc): New mode attribute.
(mov<mode>_speculative_a, zero_extend<mode>di2_speculative_a,
mov<mode>_nc, zero_extend<mode>di2_nc,
advanced_load_check_nc_<mode>): New insns.
(zero_extend*): Add speculable* attributes.
* config/ia64/ia64.opt (msched_fp_mem_deps_zero_cost): New option.
(msched-stop-bits-after-every-cycle): Likewise.
(mstop-bit-before-check): Likewise.
(msched-max-memory-insns,
msched-max-memory-insns-hard-limit): Likewise.
(msched-spec-verbose, msched-prefer-non-data-spec-insns,
msched-prefer-non-control-spec-insns,
msched-count-spec-in-critical-path,
msel-sched-renaming, msel-sched-substitution,
msel-sched-data-spec,
msel-sched-control-spec, msel-sched-dont-check-control-spec):
Use Target
Report Var instead of Common Report Var.
* config/ia64/itanium2.md: Remove strange bypass.
* config/ia64/t-ia64 (ia64.o): Add dependency on sel-sched.h.
* config/rs6000/rs6000.c (rs6000_init_sched_context,
rs6000_alloc_sched_context, rs6000_set_sched_context,
rs6000_free_sched_context): New functions.
(struct _rs6000_sched_context): New.
(rs6000_sched_reorder2): Do not modify INSN_PRIORITY for selective
scheduling.
(rs6000_sched_finish): Do not run for selective scheduling.
[-- Attachment #2: sel-sched-target.diff.gz --]
[-- Type: application/gzip, Size: 21401 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
2008-08-22 16:04 ` Andrey Belevantsev
@ 2008-09-25 22:39 ` sje
2008-09-26 14:57 ` Andrey Belevantsev
0 siblings, 1 reply; 8+ messages in thread
From: sje @ 2008-09-25 22:39 UTC (permalink / raw)
To: abel; +Cc: gcc-patches, wilson, vmakarov
Andrey,
I have started looking at the IA64 specific parts of the selective
scheduling branch. I still need some more time but I was wondering if you
could update it so that it is up-to-date with respect to the main trunk.
I tried to apply the patch so I could look at some of the changes with
more context and ia64.c would not apply cleanly.
Here are a few minor comments from what I have reviewed so far. I didn't
include the patch and put the comments inline since the patch is so
large and I only had a few comments.
There are some places where lines start with spaces instead of tabs even
though they are indented enough to use tabs and a couple of functions
(ia64_sched_init and ia64_sched_final) had lines where the only change
was from tabs to spaces.
In ia64_clear_sched_context we free _sc->prev_cycle_state, I was
wondering if we should set it to NULL after freeing it. Or are
we going to free _sc right after this so that it doesn't matter?
Is the mflag_sched_spec_verbose flag really needed? It looks like
all it does is dump output to stderr intead of the normal dump file.
In get_mode_no_for_insn, there is a check:
(AR_CCV_REGNUM <= REGNO (reg) && REGNO (reg) <= AR_EC_REGNUM)
I think this should be replaced with AR_REGNO_P ().
Steve Ellcey
sje@cup.hp.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
2008-09-25 22:39 ` sje
@ 2008-09-26 14:57 ` Andrey Belevantsev
2008-10-03 22:22 ` Steve Ellcey
0 siblings, 1 reply; 8+ messages in thread
From: Andrey Belevantsev @ 2008-09-26 14:57 UTC (permalink / raw)
To: Steve Ellcey; +Cc: gcc-patches, wilson, vmakarov, Alexander Monakov
sje@cup.hp.com wrote:
> I have started looking at the IA64 specific parts of the selective
> scheduling branch. I still need some more time but I was wondering if you
> could update it so that it is up-to-date with respect to the main trunk.
> I tried to apply the patch so I could look at some of the changes with
> more context and ia64.c would not apply cleanly.
We (Alexander and myself) just did it, so current sel-sched branch has
the version of config/ia64/* files that we'd like to see on trunk.
> There are some places where lines start with spaces instead of tabs even
> though they are indented enough to use tabs and a couple of functions
> (ia64_sched_init and ia64_sched_final) had lines where the only change
> was from tabs to spaces.
This is fixed on the branch.
> In ia64_clear_sched_context we free _sc->prev_cycle_state, I was
> wondering if we should set it to NULL after freeing it. Or are
> we going to free _sc right after this so that it doesn't matter?
Sometimes we free it and sometimes we don't, but I agree with you that
it would be clearer to set it to NULL. I will prepare a patch and check
it in the branch.
> Is the mflag_sched_spec_verbose flag really needed? It looks like
> all it does is dump output to stderr intead of the normal dump file.
No, it is not needed, but it is also not present on the branch. We have
reviewed the other introduced flags. We will remove -msel-sched* flags
related to speculation, as we can use the existing flags, and we will
remove mstop-bit-before-check flag, as it doesn't improve performance in
average.
> In get_mode_no_for_insn, there is a check:
>
> (AR_CCV_REGNUM <= REGNO (reg) && REGNO (reg) <= AR_EC_REGNUM)
>
> I think this should be replaced with AR_REGNO_P ().
Fixed on the branch. Thanks a lot for your efforts!
Andrey
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
2008-09-26 14:57 ` Andrey Belevantsev
@ 2008-10-03 22:22 ` Steve Ellcey
2008-10-06 17:26 ` Andrey Belevantsev
0 siblings, 1 reply; 8+ messages in thread
From: Steve Ellcey @ 2008-10-03 22:22 UTC (permalink / raw)
To: Andrey Belevantsev; +Cc: gcc-patches, wilson, vmakarov, Alexander Monakov
On Fri, 2008-09-26 at 17:05 +0400, Andrey Belevantsev wrote:
> sje@cup.hp.com wrote:
> > I have started looking at the IA64 specific parts of the selective
> > scheduling branch. I still need some more time but I was wondering if you
> > could update it so that it is up-to-date with respect to the main trunk.
> > I tried to apply the patch so I could look at some of the changes with
> > more context and ia64.c would not apply cleanly.
> We (Alexander and myself) just did it, so current sel-sched branch has
> the version of config/ia64/* files that we'd like to see on trunk.
>
> Andrey
Andrey,
I have reviewed the changes on the sel-sched-branch and approve the IA64
specific changes. I noticed there were a few non-IA64 changes on the branch
and obviously I can't approve those but the IA64 changes look OK.
Steve Ellcey
sje@cup.hp.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Selective scheduling pass - target changes (ia64 & rs6000) [3/3]
2008-10-03 22:22 ` Steve Ellcey
@ 2008-10-06 17:26 ` Andrey Belevantsev
0 siblings, 0 replies; 8+ messages in thread
From: Andrey Belevantsev @ 2008-10-06 17:26 UTC (permalink / raw)
To: sje; +Cc: gcc-patches, wilson, vmakarov, Alexander Monakov
Steve Ellcey wrote:
> I have reviewed the changes on the sel-sched-branch and approve the IA64
> specific changes. I noticed there were a few non-IA64 changes on the branch
> and obviously I can't approve those but the IA64 changes look OK.
Thanks for the review! I will retest the IA64 changes over the next few
days and commit. The non-IA64 changes should be just the difference
between scheduling hooks due to the changes we handle speculation. Vlad
had already approved those with the main part of sel-sched patch, I
just had to revert that pieces when committing the main part without the
IA64 part. Of course, if there are other changes, such as bugfixes not
yet approved for mainline, I will not commit them.
Thanks again,
Andrey
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-10-06 17:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-29 15:10 Selective scheduling pass - target changes (ia64 & rs6000) [3/3] David Edelsohn
2008-08-31 13:35 ` Andrey Belevantsev
-- strict thread matches above, loose matches on Subject: below --
2008-06-03 14:24 [RFC] Selective scheduling pass Andrey Belevantsev
2008-06-03 14:28 ` Selective scheduling pass - target changes (ia64 & rs6000) [3/3] Andrey Belevantsev
2008-08-22 16:04 ` Andrey Belevantsev
2008-09-25 22:39 ` sje
2008-09-26 14:57 ` Andrey Belevantsev
2008-10-03 22:22 ` Steve Ellcey
2008-10-06 17:26 ` Andrey Belevantsev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).