Re: Scheduler: Allow breaking dependencies by modifying patterns
On Fri, Aug 3, 2012 at 5:05 AM, Bernd Schmidt ber...@codesourcery.com wrote: This patch allows us to change rn++ rm=[rn] into rm=[rn + 4] rn++ Opportunities to do this are discovered by a mini-pass over the instructions after generating dependencies and before scheduling a block. At that point we have all the information required to ensure that a candidate dep between two instructions is only used to show the register dependence, and to ensure that every insn with a memory reference is only subject to at most one dep causing a pattern change. The dep_t structure is extended to hold an optional pointer to a replacement description, which holds information about what to change when a dependency is broken. The time when this replacement is applied differs depending on whether the changed insn is the DEP_CON (in which case the pattern is changed whenever the broken dependency becomes the last one), or the DEP_PRO, in which case we make the change when the corresponding DEP_CON has been scheduled. This ensures that the ready list always contains insns with the correct pattern. A few additional bits are needed in the dep structure: one to hold information about whether a dependency occurs multiple times, and one to distinguish dependencies that are purely for register values from those with other meanings (e.g. memory references). Also, sched-rgn was changed to use a new bit, DEP_POSTPONED, rather than HARD_DEP to indicate that we don't want to schedule an insn in the current block. A possible future extension would be to also allow autoinc addressing modes as the increment insn. Bootstrapped and tested on x86_64-linux, and also tested on c6x-elf (quite a number of changes were necessary to make it work there). It was originally written for a mips target and tested there in the context of a 4.6 tree. I've also run spec2000 on x86_64, with no change that looked like anything other than noise. Ok? This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54645 -- H.J.
Re: Scheduler: Allow breaking dependencies by modifying patterns
On 09/20/2012 07:42 PM, H.J. Lu wrote: On Fri, Aug 3, 2012 at 5:05 AM, Bernd Schmidt ber...@codesourcery.com wrote: This patch allows us to change rn++ rm=[rn] into rm=[rn + 4] rn++ This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54645 The jury is still out on that, but it did cause PR54643. This seems to be some kind of interaction with the new SCHED_PRESSURE_MODEL code. For now it seems best to just disable the new optimization if scheduling for pressure. It will still trigger during sched2. I've bootstrapped the following on x86_64-linux and verified that the arm build gets further (it now fails in libbacktrace). Committed. Bernd Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 191592) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2012-09-20 Bernd Schmidt ber...@codesourcery.com + + PR bootstrap/54643 + * haifa-sched.c (schedule_block): Skip find_modifiable_mems if using + SCHED_PRESSURE_MODEL. + 2012-09-20 Joern Rennecke joern.renne...@embecosm.com * rtlanal.c (dead_or_set_regno_p): Fix COND_EXEC handling. Index: gcc/haifa-sched.c === --- gcc/haifa-sched.c (revision 191582) +++ gcc/haifa-sched.c (working copy) @@ -5854,7 +5854,8 @@ schedule_block (basic_block *target_bb) rtx head = NEXT_INSN (prev_head); rtx tail = PREV_INSN (next_tail); - if ((current_sched_info-flags DONT_BREAK_DEPENDENCIES) == 0) + if ((current_sched_info-flags DONT_BREAK_DEPENDENCIES) == 0 + sched_pressure != SCHED_PRESSURE_MODEL) find_modifiable_mems (head, tail); /* We used to have code to avoid getting parameters moved from hard
Re: Scheduler: Allow breaking dependencies by modifying patterns
On 12/09/2012, at 4:34 AM, Vladimir Makarov wrote: On 08/03/2012 08:05 AM, Bernd Schmidt wrote: ... Ok, thanks. The changes are pretty straightforward. Only just a few comments. ... Thanks for the patch, Bernd. Sorry for the delay with the review. I thought that Maxim writes his comments first. I reviewed the patch in http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01028.html (comments were similar to yours), but didn't CC you. Anyway, thanks for the review! -- Maxim Kuvyrkov CodeSourcery / Mentor Graphics
Re: Scheduler: Allow breaking dependencies by modifying patterns
On 08/03/2012 08:05 AM, Bernd Schmidt wrote: This patch allows us to change rn++ rm=[rn] into rm=[rn + 4] rn++ That is an interesting optimization. I think analogous optimization could be done for INC/DEC addressing (probably it might be beneficial for ppc which has such addressing and displacement addressing). Although it will complicate the haifa scheduler quite a lot as a new insn is generated and the real benefits are may be not worth of it (as an additional insn should be generated which in many cases it could result even in worse code). Opportunities to do this are discovered by a mini-pass over the instructions after generating dependencies and before scheduling a block. At that point we have all the information required to ensure that a candidate dep between two instructions is only used to show the register dependence, and to ensure that every insn with a memory reference is only subject to at most one dep causing a pattern change. The dep_t structure is extended to hold an optional pointer to a replacement description, which holds information about what to change when a dependency is broken. The time when this replacement is applied differs depending on whether the changed insn is the DEP_CON (in which case the pattern is changed whenever the broken dependency becomes the last one), or the DEP_PRO, in which case we make the change when the corresponding DEP_CON has been scheduled. This ensures that the ready list always contains insns with the correct pattern. A few additional bits are needed in the dep structure: one to hold information about whether a dependency occurs multiple times, and one to distinguish dependencies that are purely for register values from those with other meanings (e.g. memory references). Also, sched-rgn was changed to use a new bit, DEP_POSTPONED, rather than HARD_DEP to indicate that we don't want to schedule an insn in the current block. A possible future extension would be to also allow autoinc addressing modes as the increment insn. Bootstrapped and tested on x86_64-linux, and also tested on c6x-elf (quite a number of changes were necessary to make it work there). It was originally written for a mips target and tested there in the context of a 4.6 tree. I've also run spec2000 on x86_64, with no change that looked like anything other than noise. Ok? Ok, thanks. The changes are pretty straightforward. Only just a few comments. One is a missed change log entry for haifa_note_dep. Second one is for + /* Cached cost of the dependency. Make sure to update UNKNOWN_DEP_COST + when changing the size of this field. */ + int cost:20; }; +#define UNKNOWN_DEP_COST (-119) + You could use a macro to define bit widths and UNKNOWN_DEP_COST. But probably it is a taste matter. The third one is success_in_block in find_modifiable_mems. It is calculated but nowhere used. Probably it was used for debugging. You should something to do with this. Thanks for the patch, Bernd. Sorry for the delay with the review. I thought that Maxim writes his comments first.
Re: Scheduler: Allow breaking dependencies by modifying patterns
On 08/03/2012 02:05 PM, Bernd Schmidt wrote: This patch allows us to change rn++ rm=[rn] into rm=[rn + 4] rn++ Ping. Bernd
Re: Scheduler: Allow breaking dependencies by modifying patterns
On 4/08/2012, at 12:05 AM, Bernd Schmidt wrote:
This patch allows us to change
rn++
rm=[rn]
into
rm=[rn + 4]
rn++
The patch is OK with the following nitpicks.
[BTW, if anyone else wants to review this patch, it helps to read it from the
end.]
Opportunities to do this are discovered by a mini-pass over the
instructions after generating dependencies and before scheduling a
block. At that point we have all the information required to ensure that
a candidate dep between two instructions is only used to show the
register dependence, and to ensure that every insn with a memory
reference is only subject to at most one dep causing a pattern change.
The dep_t structure is extended to hold an optional pointer to a
replacement description, which holds information about what to change
when a dependency is broken. The time when this replacement is applied
differs depending on whether the changed insn is the DEP_CON (in which
case the pattern is changed whenever the broken dependency becomes the
last one), or the DEP_PRO, in which case we make the change when the
corresponding DEP_CON has been scheduled. This ensures that the ready
list always contains insns with the correct pattern.
A few additional bits are needed in the dep structure: one to hold
information about whether a dependency occurs multiple times, and one to
distinguish dependencies that are purely for register values from those
with other meanings (e.g. memory references).
Also, sched-rgn was changed to use a new bit, DEP_POSTPONED, rather than
HARD_DEP to indicate that we don't want to schedule an insn in the
current block.
A possible future extension would be to also allow autoinc addressing
modes as the increment insn.
Would you please copy the above to a comment describing how the new
optimization fits within the scheduler? In sched-deps.c just before definition
of struct mem_inc_info seems like a good spot for an overview comment.
Bootstrapped and tested on x86_64-linux, and also tested on c6x-elf
(quite a number of changes were necessary to make it work there). It was
originally written for a mips target and tested there in the context of
a 4.6 tree. I've also run spec2000 on x86_64, with no change that looked
like anything other than noise. Ok?
+ We also perform pattern replacements for predication, and for broken
+ replacement dependencies. The latter is only done if FOR_BACKTRACK is
+ false. */
We also perform pattern replacement for ?speculation? ...
+void
+find_modifiable_mems (rtx head, rtx tail)
+{
+ rtx insn;
+ int success_in_block = 0;
Success_in_block is not really used. Maybe add a debug printout for it?
+ /* Dependency major type. This field is superseded by STATUS below.
+ Though, it is still in place because some targets use it. */
+ ENUM_BITFIELD(reg_note) type:6;
... is superseded by STATUS *above*.
--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics
Re: Scheduler: Allow breaking dependencies by modifying patterns
On 4/08/2012, at 12:05 AM, Bernd Schmidt wrote: This patch allows us to change rn++ rm=[rn] into rm=[rn + 4] rn++ This is a good scheduler optimization that I wanted to have for quite a while. Bernd, kudos for implementing it! I am going to review this patch, and then the scheduler maintainers will have an option of accepting my word for it, or doing a second review. I have reviewed earlier versions of this patch, so I know I'm happy with the general structure of the optimization. -- Maxim Kuvyrkov CodeSourcery / Mentor Graphics
