Hello,
Thanks for the comments! I incorporated them in the attached patch.
Currently testing and bootstrap with the other patch in the series on
ppc64-redhat-linux, enabling SMS on loops with SC 1.
Thanks again,
Revital
2012-01-01 Richard Sandiford
Revital Eres
* loop
Hello,
> Yes, thanks for catching this. Shouldn't we prevent creating such
> regmoves for (the other case of) intra-loop anti-deps as well?
Right! sorry for missing that. I added an additional check in
create_ddg_dep_from_intra_loop_link.
Also, thanks to Bernhard Rosenkraenzer for opening PR 879
Hello,
[PATCH, SMS] Prevent the creation of reg-moves for definitions with MODE_CC
http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01459.html
[PATCH SMS 2/2, RFC] Register pressure estimation for the partial schedule
http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01330.html
Thanks,
Revital
Hello,
Following Richard's comment
http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01469.html attached is
a new version of the patch to prevent reg-moves for non allocatable definitions.
Currently testing and bootstrap on ppc64-redhat-linux, enabling SMS on
loops with SC 1.
OK for 4.7 once testing
Hello,
> FWIW, an alternative might be to test have_regs_of_mode[(int) mode].
> That says whether there are any allocatable (non-fixed) registers
> of the given mode.
Thanks, I'll prepare a new version of the patch using have_regs_of_mode.
Revital
>
> Richard
Hello,
The testcase attached causes ICE when compiling with
-fmodulo-sched-allow-regmoves on ARM due to reg-moves created for the
definition of mode MODE_CC.
The following is a snippet from the ddg of the definition and use of vfpcc
which triggers the creation of the reg-move:
Node num: 1
(insn
Hello,
>> This patch support the estimation of register pressure in SMS.
>> Although GCC is in stage 3 I would appreciate comments on it.
>> Thanks to Richard and Ayal for discussing the implementation and their
>> insights.
>>
>> This part of the patch enables iterating on the partial schedule i
Revital Eres
* loop-invariant.c (get_regno_pressure_class): Move function to...
* ira.c: Here.
* common.opt (fmodulo-sched-reg-pressure, -fmodulo-sched-verbose):
New flags.
* doc/invoke.texi (fmodulo-sched-reg-pressure,
-fmodulo-sched-verbose
Hello,
> OK for 3.7?
Sorry, I meant GCC 4.7.0...
Thanks,
Revital
Hello,
The patch below adds a missing free operation in mark_loop_unsched.
Tested (bootstrap and regtest) ppc64-redhat-linux.
OK for 3.7?
Thanks,
Revital
Changelog:
* modulo-sched.c (mark_loop_unsched): Free bbs.
Index: modulo-sched.c
Hello,
Ping: http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02444.html
Thanks,
Revital
Hello,
Attached is a new version of the patch.
Thanks to Dominique Dhumieres for testing on
powerpc-apple-darwin9.
Tested ppc64-redhat-linux on with both -m32,-m64 and SPU.
OK for mainline?
Thanks,
Revital
testsuite/Changelog
PR rtl-optimization/47013
* gcc.dg/sms-2.c: Change
Hello,
The attached patch adds register pressure estimation of the partial schedule.
Tested and bootstrap with the other patches in the series on
ppc64-redhat-linux,
enabling SMS on loops with SC 1.
Comments are welcome.
Thanks,
Revital
Changelog:
* loop-invariant.c (get_regno_pressure
Hello,
This patch support the estimation of register pressure in SMS.
Although GCC is in stage 3 I would appreciate comments on it.
Thanks to Richard and Ayal for discussing the implementation and their insights.
This part of the patch enables iterating on the partial schedule in the
reverse orde
Hello,
>> Tested and bootstrap on all languages except java (PR50879) on
>> ppc64-redhat-linux, enabling SMS on loops with SC 1.
>>
>> OK for mainline?
>>
>
> OK, seems reasonable.
>
> Please fix typo (in original comment):
> - scheduling passes doesn't touch it. */
> + sche
Hello,
The attach patch fixes the current marking of SMS loops to prevent
further scheduling as follows: it marks *all* the loop's bbs with
BB_DISABLE_SCHEDULE which prevents them from been scheduled later.
(with the current implementation if the loop has non empty latch then
it will be considered
Hello,
> OK, with the following comments:
>
> Make sure reg_moves are generated for the correct (result, not addr)
> register, in generate_reg_moves().
>
> "been">>"being" (multiple appearances).
>
> Add a note that autoinc_var_is_used_p (rtx def_insn, rtx use_insn)
> doesn't need to consider the
Hello,
> This
> + /* Skip instructions that do not set a register. */
> + if (set && !REG_P (SET_DEST (set)))
> + continue;
> is ok. Can you also prevent !set insns from having reg_moves? (To be updated
> once auto_inc insns will be supported, if they'll deserve reg_moves too.)
Hello,
> Ok, this does have two anti-dep edges. But still, only a single true
> dependence(?) ... can you see why?
The intra edge [3681 -(T,2,0)-> 3682] was created by haifa-sched and I guess
that because both of the expected true-dep edges (one for the target
and one for the address) are identic
Hello,
> This
> + /* Skip instructions that do not set a register. */
> + if (set && !REG_P (SET_DEST (set)))
> + continue;
> is ok. Can you also prevent !set insns from having reg_moves? (To be updated
> once auto_inc insns will be supported, if they'll deserve reg_moves too.)
Hello,
> ok, so if we have an auto-inc'ing insn which defines (auto-inc's) an
> addr register and another (say, result) register, we want to allow the
> result register to have life ranges in excess of ii (by eliminating
> anti-dep edges of distance 1 from uses to def, and generating
> reg_moves i
Hello,
This patch extends the implementation to support instructions with
REG_INC notes.
It addresses the comments from the previous submission:
http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01299.html.
btw, regarding your previous question about the usage of the address
register been auto inc, ap
Hello,
The attached patch contains a fix to generate_reg_moves
function. Currently we can generate reg-moves for stores which are later
eliminated. This happens when we have mem dependency with distance 1
and as a result the number of regmoves is at least 1 based on the
following
calculation take
Hello,
> OK.
> While we're at it, an alternative would be to have
> remove_node_from_ps() assert its own (parameters and) return value.
> That is, replace "if (c) return false" by "assert (!c)" and have it
> return void if successful. There's not much you can do if it returns
> false. That would c
Hello,
The attached patch contains minor fixes.
Currently testing and bootstrap on ppc64-redhat-linux enabling SMS on
loops with SC 1.
OK for mainline once testing completes?
Thanks,
Revital
Changelog
* modulo-sched.c (optimize_sc): Call remove_node_from_ps outside
of gcc_ass
Hello,
On 16 August 2011 03:32, Ayal Zaks wrote:
> Ok, so this extends the infrastructure to support insns which set an
> arbitrary number of registers, but currently specifically handles only
> REG_INC situations (which set two registers). I'm not against
> {0,1,infinity}, but wonder if this cas
Hello,
This is a re-submission of the patch to support instructions with REG_INC_NOTE.
(http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01309.html)
It contains a minor change from the previous submission suggested by
Richard Sandiford: to use reg_referenced_p instead of
rtx_referenced_p.
The patch
Hello,
[PATCH, SMS 3/4] Optimize stage count
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01341.html
[PATCH, SMS 4/4] Misc. fixes
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01342.html
[PATCH, SMS] Fix calculation of issue_rate
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01344.html
Thanks,
Re
Hello,
>>> * ddg.c (add_intra_loop_mem_dep): New function.
>
> You could check first thing if (from->cuid == to->cuid), for code clarity.
Attached is the new version of the patch which addresses this.
The patch was re-tested as follows:
On ppc64-redhat-linux regtest as well as bootstrap
Hello,
> better do
> else if (!mem_read_insn_p (to->insn))
>
> + create_ddg_dep_no_link (g, from, to, ANTI_DEP, MEM_DEP, 0);
> + }
Done. Committed to -r175090.
Thanks,
Revital
Hello,
>> You could check first thing if (from->cuid == to->cuid), for code clarity.
>
> I will address this point separately and commit the current version of
> the patch as is if that's OK.
Re-thinking about that, I'll prepare a new version of the patch which
addresses this and re-send it.
Sor
Hello,
> Yes, this is a straightforward fix to a wrong-code bug, as discussed
> offline. Other alternatives that might introduce less edges:
> o connect predecessors of u with v, and u with successors of v, when
> removing edge (u,v). Maybe there are other cases which rely on transitivity
> (?).
Hello,
> Please add the following:
> o A clarification that rows_length is used only (as an optimization) to
> back off quickly from trying to schedule a node in a full row; that is, to
> avoid running through futile DFA state transitions.
> o An assert that ps->rows_length[i] equals the number of
Hello,
The attached patch fixes violation of memory dependencies. The
problematic scenario happens when -fmodulo-sched-allow-regmoves flag
is set and certain anti-dep edges are not created.
For example, consider the following three instructions and the edges
between them. When -fmodulo-sched-all
Hello,
>
> OK, this makes sense. Just to re-confirm, the exact same edges are created
> in both cases, right?
No, sorry for not been more clear on this. The previous implementation
will actually create redundant anti-dep edges. This happens when
the definition that is used in jump_insn is define
Hello,
> Please add the following:
> o A clarification that rows_length is used only (as an optimization) to
> back off quickly from trying to schedule a node in a full row; that is, to
> avoid running through futile DFA state transitions.
> o An assert that ps->rows_length[i] equals the number of
Hello,
[PATCH, SMS 1/4] Fix calculation of row_rest_count
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01339.html
[PATCH, SMS 2/4] Move the creation of anti-dep edge
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01340.html
[PATCH, SMS 3/4] Optimize stage count
http://gcc.gnu.org/ml/gcc-patches/20
Hello,
>
> * modulo-sched.c (sms_schedule): Fix stage_count calculation.
>
Sorry, just noticed that the changelog entry is wrong, here
a correction:
* modulo-sched.c (sms_schedule): Fix issue_rate calculation.
Thanks,
Revital
Hello,
The issue rate is currently been set in SMS by calling
targetm.sched.issue_rate function if it is defined. For rs6000 the
issue_rate is 1 if !reload_completed && !flag_sched_pressure.
To bypass that, SMS sets reload_completed to 1 before calling
targetm.sched.issue_rate and restores it's
Hello,
The attached patch contains misc. fixes and changes.
The patch was tested together with the rest of the patches in this series.
On ppc64-redhat-linux regtest as well as bootstrap with SMS flags
enabling SMS also on loops with stage count 1. Regtested on SPU.
On arm-linux-gnueabi regtseted
Hello,
The attach patch tries to achieve optimised SC by normalizing the partial
schedule (having the cycles start from cycle zero). The branch location
must be placed in row ii-1 in the final scheduling. If that's not the
case after the normalization then it tries to move the branch to that
row
Hello,
The attached patch moves the creation of anti-dep edge from a
branch to it's def from create_ddg_dep_from_intra_loop_link () to
add_cross_iteration_register_deps () due to the fact the edge is with
distance 1 and thus should be in the later function.
The edge was added to avoid creating re
Hello,
The calculation of the number of instructions in a row is currently
done by updating row_rest_count field in struct ps_insn on the fly
while creating a new instruction. It is used to make sure we do not
exceed
the issue_rate.
This calculation assumes the instruction is inserted in the begi
Hello,
Attached is a new version of the patch.
Thanks to Dominique Dhumieres for testing on
powerpc-apple-darwin9 and x86_64-apple-darwin10.
Tested ppc64-redhat-linux on both with -m32,-m64 and SPU.
OK for mainline?
Thanks,
Revital
testsuite/Changelog
PR rtl-optimization/47013
Hello,
Thanks for testing the patch.
> FAIL: gcc.dg/sms-8.c scan-rtl-dump-times sms "SMS loop with subreg in lhs" 1
Does the attached patch resolve the failure with sms-8.c?
If so I'll re-submit it.
Thanks,
Revital
Index: testsuite/gcc.dg/sms-2.c
Hello Ramana,
Following our conversation; here is the patch again without
the arm specific flags. Tested on ARM machine configured with
[--with-arch=armv7-a] --with-mode=thumb.
OK for mainline?
Thanks,
Revital
testsuite/Changlog:
* gcc.target/arm/sms-9.c: New file.
* gcc.target
Hello,
The attached patch fixes SMS testsuite failures seen on PowerPC and SPU.
Tested ppc64-redhat-linux on both with -m32,-m64 and SPU.
OK for mainline?
Thanks,
Revital
testsuite/Changelog
PR rtl-optimization/47013
* gcc.dg/sms-2.c: Change scan-tree-dump-times and the code i
Hello,
>> + if (reg_mentioned_p (reg, insn) && !DEBUG_INSN_P (insn))
>
> It probably makes sense to test for !DEBUG_INSN_P first, since it's much
> cheaper.
Thanks, will commit the following to fix that:
* modulo-sched.c (doloop_register_get): Check !DEBUG_INSN_P
first.
Inde
Hello,
> please clarify that, e.g., instruction scheduled on cycle AMOUNT will move
> to cycle zero.
OK, done.
> shouldn't normalized_time be used here instead of SCHED_TIME (u)?
SCHED_TIME (u) is been set to normalized_time just before
using it.
Thanks,
Revital
Hello,
> please clarify that, e.g., instruction scheduled on cycle AMOUNT will move
> to cycle zero.
OK, done.
> shouldn't normalized_time be used here instead of SCHED_TIME (u)?
SCHED_TIME (u) is been set to normalized_time just before
using it.
Thanks,
Revital
Hello,
(sorry for multiple copies of this email)
This small fix was inserted to skip DEBUG_INSNs while
recognizing doloop pattern in loop-doloop.c file. It's a fix
for the already approved do-loop patch (not in mainline yet,
http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01718.html) in loop-doloop
Hello,
(sorry for multiple copies of this email)
This small fix was inserted to skip DEBUG_INSNs while
recognizing doloop pattern in loop-doloop.c file. It's a fix
for the already approved do-loop patch (not in mainline yet,
http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01718.html) in loop-doloop
Hello,
The attached patch adds code to skip DEBUG_INSNs while recognizing
doloop pattern.
The patch was tested together with the rest of the patches in this series
and on top of the patch to support do-loop for ARM (not yet in mainline,
but approved http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01
Hello,
The attached patch includes enhancements for SMS to support targets
that their doloop part is not decoupled from the rest of the loop's
instructions, as SMS currently requires. In this case, the branch can
not be placed wherever we want (as is currently done) due to the fact
it must honor d
My apologies, the previous email refers to the patch:
http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00350.html
- Support closing_branch_deps
and not to 'Avoid considering debug_insn when calculating SCCs' as
the title implies.
Thanks,
Revital
Hello,
The following is a summary of discussion I had with Ayal regarding the patch:
Some background: currently, SMS supports only targets where the doloop
pattern is decoupled from the rest of the loop's instructions (for example
PowerPC) (we'll call it 'case decoupled' for simplicity) In this
Hello,
http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00350.html
- Support closing_branch_deps
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01309.html
- Support instructions with REG_INC_NOTE
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01294.html
- Avoid considering debug_insn when calculating SC
Hello,
The attached patch extends the current implementation to analyze
instructions with REG_INC_NOTE.
Tested on ppc64-redhat-linux (bootstrap and regtest) SPU (only regtest)
and arm-linux-gnueabi (bootstrap c and regtest) configured with
--with-arch=armv7-a --with-mode=thumb.
OK for mainline?
Hello,
>
> New params need documentation in doc/invoke.texi. Please also change the
> maximum value to 2, not 1. Ok with that changes.
When changing the max value to 2, any value that is greater than
2 is rejected due to the following comment in params.def:
- The maximum acceptable value
Hello,
The attached patch avoids considering debug_insn when calculating SCCs.
With this change the existence of debug_insn does not influence
the scheduling order and rec_MII.
Bootstrap and regtest on ppc64-redhat-linux and regtest on arm-linux-gnueabi.
OK for mainline?
Thanks,
Revital
Change
Hello,
Following Richard's suggestion here is the patch again.
Tested on ppc64-redhat-linux.
OK for mainline?
Thanks,
Revital
Changelog
* params.def (sms-min-sc): New param flag.
* modulo-sched.c (sms_schedule): Use it.
=== modified file 'gcc/modulo-sched.c'
--- gcc/modulo-sch
Hello,
> If it's for debugging, can you use a --parm instead (like
> modulo-sched-min-sc or similar)?
>
I think I can use --param for debugging purposes in this case.
(I might add modulo-sched-max-sc as well)
Thanks,
Revital
> Thanks,
> Richard.
>
>> Thanks,
>> Revital
>>
>> Changelog:
>>
>>
Hello,
On 15 April 2011 18:53, Nathan Froyd wrote:
> On Fri, Apr 15, 2011 at 06:27:05PM +0300, Revital Eres wrote:
>> + if (all_sccs->sccs)
>> + free (all_sccs->sccs);
>
> No need to check for non-NULL prior to free'ing.
OK, I'll commit the patch with
Hello,
The attached patch introduces a new flag to allow applying SMS when
stage count (SC) also equals 1.
Currently, SMS is applied only when SC greater than 1 as stage count
of 1 means that there is no interleaving between iterations and the
scheduling passes do the job in this case.
The new fla
Hello,
The attached patch adds missing free operation for storage
allocated while calculating SCCs.
Bootstrap and regtested on ppc64-redhat-linux.
OK for mainline?
Thanks,
Revital
Changelog:
* ddg.c (free_ddg_all_sccs): Free sccs field in struct ddg_all_sccs.
Index: ddg.c
===
Hello,
This patch fixes the scenario where SMS fails to
schedule a loop and continue to the next one without
freeing data structures allocated while scheduling
the first loop.
Bootstrap and regtested on ppc64-redhat-linux.
OK for mainline?
Thanks,
Revital
Changelog:
* modulo-sched.c (sms_sche
66 matches
Mail list logo
