* [PATCH] Guard against applying scale with 0 denominator @ 2022-05-06 20:31 Eugene Rozenfeld 2022-05-09 7:42 ` Richard Biener 0 siblings, 1 reply; 6+ messages in thread From: Eugene Rozenfeld @ 2022-05-06 20:31 UTC (permalink / raw) To: gcc-patches Calling count.apply_scale with a 0 denominator causes an assert. This change guards against that. Tested on x86_64-pc-linux-gnu. gcc/ChangeLog: * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying scale with 0 denominator. --- gcc/tree-vect-loop-manip.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc index 1d4337eb261..db54ae69e45 100644 --- a/gcc/tree-vect-loop-manip.cc +++ b/gcc/tree-vect-loop-manip.cc @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, get lost if we scale down to 0. */ basic_block *bbs = get_loop_body (epilog); for (unsigned int i = 0; i < epilog->num_nodes; i++) - bbs[i]->count = bbs[i]->count.apply_scale - (bbs[i]->count, - bbs[i]->count.apply_probability - (prob_vector)); + if (bbs[i]->count.nonzero_p ()) + bbs[i]->count = bbs[i]->count.apply_scale + (bbs[i]->count, + bbs[i]->count.apply_probability + (prob_vector)); free (bbs); } -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Guard against applying scale with 0 denominator 2022-05-06 20:31 [PATCH] Guard against applying scale with 0 denominator Eugene Rozenfeld @ 2022-05-09 7:42 ` Richard Biener 2022-05-12 1:37 ` [EXTERNAL] " Eugene Rozenfeld 0 siblings, 1 reply; 6+ messages in thread From: Richard Biener @ 2022-05-09 7:42 UTC (permalink / raw) To: Eugene Rozenfeld, Jan Hubicka; +Cc: gcc-patches On Fri, May 6, 2022 at 10:32 PM Eugene Rozenfeld via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Calling count.apply_scale with a 0 denominator causes an assert. > This change guards against that. > > Tested on x86_64-pc-linux-gnu. > > gcc/ChangeLog: > * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying scale with 0 denominator. > --- > gcc/tree-vect-loop-manip.cc | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc > index 1d4337eb261..db54ae69e45 100644 > --- a/gcc/tree-vect-loop-manip.cc > +++ b/gcc/tree-vect-loop-manip.cc > @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, > get lost if we scale down to 0. */ > basic_block *bbs = get_loop_body (epilog); > for (unsigned int i = 0; i < epilog->num_nodes; i++) > - bbs[i]->count = bbs[i]->count.apply_scale > - (bbs[i]->count, > - bbs[i]->count.apply_probability > - (prob_vector)); > + if (bbs[i]->count.nonzero_p ()) > + bbs[i]->count = bbs[i]->count.apply_scale > + (bbs[i]->count, > + bbs[i]->count.apply_probability > + (prob_vector)); So exactly what the FIXME in the comment above says happens. It might be better to save/restore the old counts if the intent is to get them back. I'm not exactly sure where the other scaling happens though. Richard. > free (bbs); > } > > -- > 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator 2022-05-09 7:42 ` Richard Biener @ 2022-05-12 1:37 ` Eugene Rozenfeld 2022-05-12 7:34 ` Richard Biener 0 siblings, 1 reply; 6+ messages in thread From: Eugene Rozenfeld @ 2022-05-12 1:37 UTC (permalink / raw) To: Richard Biener, Jan Hubicka; +Cc: gcc-patches In my case this is not exactly what the FIXME in the comment above says. The count is 0 even before the initial scaling happens. I hit this case with some changes I'm working on to enable per-instruction discriminators for AutoFDO. I looked into saving/restoring the old counts but it would be awkward to do. The initial scaling happens here: if (skip_vector) { split_edge (loop_preheader_edge (loop)); /* Due to the order in which we peel prolog and epilog, we first propagate probability to the whole loop. The purpose is to avoid adjusting probabilities of both prolog and vector loops separately. Note in this case, the probability of epilog loop needs to be scaled back later. */ basic_block bb_before_loop = loop_preheader_edge (loop)->src; if (prob_vector.initialized_p ()) { scale_bbs_frequencies (&bb_before_loop, 1, prob_vector); scale_loop_profile (loop, prob_vector, 0); } } The scaling happens before we do the cloning for the epilog so to save/restore the counts we would need to maintain a mapping between the original basic blocks and the cloned basic blocks in the epilog. I'd like to get my simple fix in since it makes things better even if it doesn't address the issue mentioned In the FIXME. -----Original Message----- From: Richard Biener <richard.guenther@gmail.com> Sent: Monday, May 09, 2022 12:42 AM To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; Jan Hubicka <hubicka@ucw.cz> Cc: gcc-patches@gcc.gnu.org Subject: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator On Fri, May 6, 2022 at 10:32 PM Eugene Rozenfeld via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Calling count.apply_scale with a 0 denominator causes an assert. > This change guards against that. > > Tested on x86_64-pc-linux-gnu. > > gcc/ChangeLog: > * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying scale with 0 denominator. > --- > gcc/tree-vect-loop-manip.cc | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc > index 1d4337eb261..db54ae69e45 100644 > --- a/gcc/tree-vect-loop-manip.cc > +++ b/gcc/tree-vect-loop-manip.cc > @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, > get lost if we scale down to 0. */ > basic_block *bbs = get_loop_body (epilog); > for (unsigned int i = 0; i < epilog->num_nodes; i++) > - bbs[i]->count = bbs[i]->count.apply_scale > - (bbs[i]->count, > - bbs[i]->count.apply_probability > - (prob_vector)); > + if (bbs[i]->count.nonzero_p ()) > + bbs[i]->count = bbs[i]->count.apply_scale > + (bbs[i]->count, > + bbs[i]->count.apply_probability > + (prob_vector)); So exactly what the FIXME in the comment above says happens. It might be better to save/restore the old counts if the intent is to get them back. I'm not exactly sure where the other scaling happens though. Richard. > free (bbs); > } > > -- > 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator 2022-05-12 1:37 ` [EXTERNAL] " Eugene Rozenfeld @ 2022-05-12 7:34 ` Richard Biener 2022-05-20 22:28 ` Eugene Rozenfeld 0 siblings, 1 reply; 6+ messages in thread From: Richard Biener @ 2022-05-12 7:34 UTC (permalink / raw) To: Eugene Rozenfeld; +Cc: Jan Hubicka, gcc-patches On Thu, May 12, 2022 at 3:37 AM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote: > > In my case this is not exactly what the FIXME in the comment above says. The count is 0 even before > the initial scaling happens. I hit this case with some changes I'm working on to enable per-instruction discriminators for AutoFDO. > > I looked into saving/restoring the old counts but it would be awkward to do. The initial scaling happens here: > > if (skip_vector) > { > split_edge (loop_preheader_edge (loop)); > > /* Due to the order in which we peel prolog and epilog, we first > propagate probability to the whole loop. The purpose is to > avoid adjusting probabilities of both prolog and vector loops > separately. Note in this case, the probability of epilog loop > needs to be scaled back later. */ > basic_block bb_before_loop = loop_preheader_edge (loop)->src; > if (prob_vector.initialized_p ()) > { > scale_bbs_frequencies (&bb_before_loop, 1, prob_vector); > scale_loop_profile (loop, prob_vector, 0); > } > } > > The scaling happens before we do the cloning for the epilog so to save/restore the counts > we would need to maintain a mapping between the original basic blocks and the cloned basic blocks in the epilog. There is one already - after the epilogue is copied you can use get_bb_original (epilouge_bb) to get at the block it was copied from. It could also be that we can rely on the basic-block order to be preserved between the copies (I _think_ that would work ... but then assert () for this using get_bb_original ()). That means the simplest fix would be to have an auto_vec<count> and initialize that from the BB counts in loop body order (we also have exactly two BBs in all peeled loops ... But note we only scaled the scalar if-converted loop but eventually used the not if-coverted copy for the epilogue (if not vectorizing the epilogue), so indiscriminate scaling back looks wrong in some cases (I'd have to double-check the details here). > I'd like to get my simple fix in since it makes things better even if it doesn't address the issue mentioned > In the FIXME. But don't you need to check that bbs[i]->count.apply_probability (prob_vector) is not zero instead of checking that bbs[i].count is not zero? Richard. > -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Monday, May 09, 2022 12:42 AM > To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; Jan Hubicka <hubicka@ucw.cz> > Cc: gcc-patches@gcc.gnu.org > Subject: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator > > On Fri, May 6, 2022 at 10:32 PM Eugene Rozenfeld via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > Calling count.apply_scale with a 0 denominator causes an assert. > > This change guards against that. > > > > Tested on x86_64-pc-linux-gnu. > > > > gcc/ChangeLog: > > * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying scale with 0 denominator. > > --- > > gcc/tree-vect-loop-manip.cc | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc > > index 1d4337eb261..db54ae69e45 100644 > > --- a/gcc/tree-vect-loop-manip.cc > > +++ b/gcc/tree-vect-loop-manip.cc > > @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, > > get lost if we scale down to 0. */ > > basic_block *bbs = get_loop_body (epilog); > > for (unsigned int i = 0; i < epilog->num_nodes; i++) > > - bbs[i]->count = bbs[i]->count.apply_scale > > - (bbs[i]->count, > > - bbs[i]->count.apply_probability > > - (prob_vector)); > > + if (bbs[i]->count.nonzero_p ()) > > + bbs[i]->count = bbs[i]->count.apply_scale > > + (bbs[i]->count, > > + bbs[i]->count.apply_probability > > + (prob_vector)); > > So exactly what the FIXME in the comment above says happens. It > might be better > to save/restore the old counts if the intent is to get them back. I'm not exactly sure where the other scaling happens though. > > Richard. > > > > > free (bbs); > > } > > > > -- > > 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator 2022-05-12 7:34 ` Richard Biener @ 2022-05-20 22:28 ` Eugene Rozenfeld 2022-05-23 10:33 ` Richard Biener 0 siblings, 1 reply; 6+ messages in thread From: Eugene Rozenfeld @ 2022-05-20 22:28 UTC (permalink / raw) To: Richard Biener; +Cc: Jan Hubicka, gcc-patches [-- Attachment #1: Type: text/plain, Size: 5029 bytes --] Thank you for the feedback Richard. I attached a patch that saves/restores counts if the epilog doesn't use a scalar loop. Eugene -----Original Message----- From: Richard Biener <richard.guenther@gmail.com> Sent: Thursday, May 12, 2022 12:34 AM To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> Cc: Jan Hubicka <hubicka@ucw.cz>; gcc-patches@gcc.gnu.org Subject: Re: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator On Thu, May 12, 2022 at 3:37 AM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote: > > In my case this is not exactly what the FIXME in the comment above > says. The count is 0 even before the initial scaling happens. I hit this case with some changes I'm working on to enable per-instruction discriminators for AutoFDO. > > I looked into saving/restoring the old counts but it would be awkward to do. The initial scaling happens here: > > if (skip_vector) > { > split_edge (loop_preheader_edge (loop)); > > /* Due to the order in which we peel prolog and epilog, we first > propagate probability to the whole loop. The purpose is to > avoid adjusting probabilities of both prolog and vector loops > separately. Note in this case, the probability of epilog loop > needs to be scaled back later. */ > basic_block bb_before_loop = loop_preheader_edge (loop)->src; > if (prob_vector.initialized_p ()) > { > scale_bbs_frequencies (&bb_before_loop, 1, prob_vector); > scale_loop_profile (loop, prob_vector, 0); > } > } > > The scaling happens before we do the cloning for the epilog so to > save/restore the counts we would need to maintain a mapping between the original basic blocks and the cloned basic blocks in the epilog. There is one already - after the epilogue is copied you can use get_bb_original (epilouge_bb) to get at the block it was copied from. It could also be that we can rely on the basic-block order to be preserved between the copies (I _think_ that would work ... but then assert () for this using get_bb_original ()). That means the simplest fix would be to have an auto_vec<count> and initialize that from the BB counts in loop body order (we also have exactly two BBs in all peeled loops ... But note we only scaled the scalar if-converted loop but eventually used the not if-coverted copy for the epilogue (if not vectorizing the epilogue), so indiscriminate scaling back looks wrong in some cases (I'd have to double-check the details here). > I'd like to get my simple fix in since it makes things better even if > it doesn't address the issue mentioned In the FIXME. But don't you need to check that bbs[i]->count.apply_probability (prob_vector) is not zero instead of checking that bbs[i].count is not zero? Richard. > -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Monday, May 09, 2022 12:42 AM > To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; Jan Hubicka > <hubicka@ucw.cz> > Cc: gcc-patches@gcc.gnu.org > Subject: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 > denominator > > On Fri, May 6, 2022 at 10:32 PM Eugene Rozenfeld via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > Calling count.apply_scale with a 0 denominator causes an assert. > > This change guards against that. > > > > Tested on x86_64-pc-linux-gnu. > > > > gcc/ChangeLog: > > * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying scale with 0 denominator. > > --- > > gcc/tree-vect-loop-manip.cc | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/tree-vect-loop-manip.cc > > b/gcc/tree-vect-loop-manip.cc index 1d4337eb261..db54ae69e45 100644 > > --- a/gcc/tree-vect-loop-manip.cc > > +++ b/gcc/tree-vect-loop-manip.cc > > @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, > > get lost if we scale down to 0. */ > > basic_block *bbs = get_loop_body (epilog); > > for (unsigned int i = 0; i < epilog->num_nodes; i++) > > - bbs[i]->count = bbs[i]->count.apply_scale > > - (bbs[i]->count, > > - bbs[i]->count.apply_probability > > - (prob_vector)); > > + if (bbs[i]->count.nonzero_p ()) > > + bbs[i]->count = bbs[i]->count.apply_scale > > + (bbs[i]->count, > > + bbs[i]->count.apply_probability > > + (prob_vector)); > > So exactly what the FIXME in the comment above says happens. It > might be better > to save/restore the old counts if the intent is to get them back. I'm not exactly sure where the other scaling happens though. > > Richard. > > > > > free (bbs); > > } > > > > -- > > 2.25.1 [-- Attachment #2: 0001-Fix-profile-count-maintenance-in-vectorizer-peeling.patch --] [-- Type: application/octet-stream, Size: 3297 bytes --] From d439b8a5ad0af8ae6b5e0086eed4eebc160fa9aa Mon Sep 17 00:00:00 2001 From: Eugene Rozenfeld <erozen@microsoft.com> Date: Tue, 26 Apr 2022 14:28:16 -0700 Subject: [PATCH] Fix profile count maintenance in vectorizer peeling. This patch changes the code to save/restore profile counts for the epliog loop (when not using scalar loop in the epilog) instead of scaling them down and then back up, which may lead to problems if we scale down to 0. Tested on x86_64-pc-linux-gnu. gcc/ChangeLog: * tree-loop-vect-manip.cc (vect_do_peeling): Save/restore profile counts for the epilog loop. --- gcc/tree-vect-loop-manip.cc | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc index a7bbc916bbc..501a6ab99f0 100644 --- a/gcc/tree-vect-loop-manip.cc +++ b/gcc/tree-vect-loop-manip.cc @@ -2803,10 +2803,21 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo)) skip_epilog = false; + class loop *scalar_loop = LOOP_VINFO_SCALAR_LOOP (loop_vinfo); + auto_vec<profile_count> original_counts; + basic_block *original_bbs = NULL; + if (skip_vector) { split_edge (loop_preheader_edge (loop)); + if (epilog_peeling && (vect_epilogues || scalar_loop == NULL)) + { + original_bbs = get_loop_body (loop); + for (unsigned int i = 0; i < loop->num_nodes; i++) + original_counts.safe_push(original_bbs[i]->count); + } + /* Due to the order in which we peel prolog and epilog, we first propagate probability to the whole loop. The purpose is to avoid adjusting probabilities of both prolog and vector loops @@ -2821,7 +2832,6 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, } dump_user_location_t loop_loc = find_loop_location (loop); - class loop *scalar_loop = LOOP_VINFO_SCALAR_LOOP (loop_vinfo); if (vect_epilogues) /* Make sure to set the epilogue's epilogue scalar loop, such that we can use the original scalar loop as remaining epilogue if necessary. */ @@ -2975,16 +2985,19 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, a merge point of control flow. */ guard_to->count = guard_bb->count; - /* Scale probability of epilog loop back. - FIXME: We should avoid scaling down and back up. Profile may - get lost if we scale down to 0. */ - basic_block *bbs = get_loop_body (epilog); - for (unsigned int i = 0; i < epilog->num_nodes; i++) - bbs[i]->count = bbs[i]->count.apply_scale - (bbs[i]->count, - bbs[i]->count.apply_probability - (prob_vector)); - free (bbs); + /* Restore the counts of the epilog loop if we didn't use the scalar loop. */ + if (vect_epilogues || scalar_loop == NULL) + { + gcc_assert(epilog->num_nodes == loop->num_nodes); + basic_block *bbs = get_loop_body (epilog); + for (unsigned int i = 0; i < epilog->num_nodes; i++) + { + gcc_assert(get_bb_original (bbs[i]) == original_bbs[i]); + bbs[i]->count = original_counts[i]; + } + free (bbs); + free (original_bbs); + } } basic_block bb_before_epilog = loop_preheader_edge (epilog)->src; -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator 2022-05-20 22:28 ` Eugene Rozenfeld @ 2022-05-23 10:33 ` Richard Biener 0 siblings, 0 replies; 6+ messages in thread From: Richard Biener @ 2022-05-23 10:33 UTC (permalink / raw) To: Eugene Rozenfeld; +Cc: Jan Hubicka, gcc-patches On Sat, May 21, 2022 at 12:28 AM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote: > > Thank you for the feedback Richard. I attached a patch that saves/restores counts if the epilog doesn't use a scalar loop. OK. Thanks, Richard. > Eugene > > -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Thursday, May 12, 2022 12:34 AM > To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> > Cc: Jan Hubicka <hubicka@ucw.cz>; gcc-patches@gcc.gnu.org > Subject: Re: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 denominator > > On Thu, May 12, 2022 at 3:37 AM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote: > > > > In my case this is not exactly what the FIXME in the comment above > > says. The count is 0 even before the initial scaling happens. I hit this case with some changes I'm working on to enable per-instruction discriminators for AutoFDO. > > > > I looked into saving/restoring the old counts but it would be awkward to do. The initial scaling happens here: > > > > if (skip_vector) > > { > > split_edge (loop_preheader_edge (loop)); > > > > /* Due to the order in which we peel prolog and epilog, we first > > propagate probability to the whole loop. The purpose is to > > avoid adjusting probabilities of both prolog and vector loops > > separately. Note in this case, the probability of epilog loop > > needs to be scaled back later. */ > > basic_block bb_before_loop = loop_preheader_edge (loop)->src; > > if (prob_vector.initialized_p ()) > > { > > scale_bbs_frequencies (&bb_before_loop, 1, prob_vector); > > scale_loop_profile (loop, prob_vector, 0); > > } > > } > > > > The scaling happens before we do the cloning for the epilog so to > > save/restore the counts we would need to maintain a mapping between the original basic blocks and the cloned basic blocks in the epilog. > > There is one already - after the epilogue is copied you can use get_bb_original (epilouge_bb) to get at the block it was copied from. > It could also be that we can rely on the basic-block order to be preserved between the copies (I _think_ that would work ... but then assert () for this using get_bb_original ()). That means the simplest fix would be to have an auto_vec<count> and initialize that from the BB counts in loop body order (we also have exactly two BBs in all peeled loops ... > > But note we only scaled the scalar if-converted loop but eventually used the not if-coverted copy for the epilogue (if not vectorizing the epilogue), so indiscriminate scaling back looks wrong in some cases (I'd have to double-check the details here). > > > I'd like to get my simple fix in since it makes things better even if > > it doesn't address the issue mentioned In the FIXME. > > But don't you need to check that > bbs[i]->count.apply_probability (prob_vector) is not zero instead of checking that bbs[i].count is not zero? > > Richard. > > > -----Original Message----- > > From: Richard Biener <richard.guenther@gmail.com> > > Sent: Monday, May 09, 2022 12:42 AM > > To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; Jan Hubicka > > <hubicka@ucw.cz> > > Cc: gcc-patches@gcc.gnu.org > > Subject: [EXTERNAL] Re: [PATCH] Guard against applying scale with 0 > > denominator > > > > On Fri, May 6, 2022 at 10:32 PM Eugene Rozenfeld via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > > > Calling count.apply_scale with a 0 denominator causes an assert. > > > This change guards against that. > > > > > > Tested on x86_64-pc-linux-gnu. > > > > > > gcc/ChangeLog: > > > * tree-loop-vect-manip.cc (vect_do_peeling): Guard against applying scale with 0 denominator. > > > --- > > > gcc/tree-vect-loop-manip.cc | 9 +++++---- > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > diff --git a/gcc/tree-vect-loop-manip.cc > > > b/gcc/tree-vect-loop-manip.cc index 1d4337eb261..db54ae69e45 100644 > > > --- a/gcc/tree-vect-loop-manip.cc > > > +++ b/gcc/tree-vect-loop-manip.cc > > > @@ -2989,10 +2989,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1, > > > get lost if we scale down to 0. */ > > > basic_block *bbs = get_loop_body (epilog); > > > for (unsigned int i = 0; i < epilog->num_nodes; i++) > > > - bbs[i]->count = bbs[i]->count.apply_scale > > > - (bbs[i]->count, > > > - bbs[i]->count.apply_probability > > > - (prob_vector)); > > > + if (bbs[i]->count.nonzero_p ()) > > > + bbs[i]->count = bbs[i]->count.apply_scale > > > + (bbs[i]->count, > > > + bbs[i]->count.apply_probability > > > + (prob_vector)); > > > > So exactly what the FIXME in the comment above says happens. It > > might be better > > to save/restore the old counts if the intent is to get them back. I'm not exactly sure where the other scaling happens though. > > > > Richard. > > > > > > > > > free (bbs); > > > } > > > > > > -- > > > 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-23 10:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-06 20:31 [PATCH] Guard against applying scale with 0 denominator Eugene Rozenfeld 2022-05-09 7:42 ` Richard Biener 2022-05-12 1:37 ` [EXTERNAL] " Eugene Rozenfeld 2022-05-12 7:34 ` Richard Biener 2022-05-20 22:28 ` Eugene Rozenfeld 2022-05-23 10:33 ` Richard Biener
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).