* [PATCH] vec: remove unreachable code
@ 2020-09-04 12:11 Andrea Corallo
2020-09-04 12:12 ` Richard Biener
2020-09-04 15:37 ` Kewen.Lin
0 siblings, 2 replies; 8+ messages in thread
From: Andrea Corallo @ 2020-09-04 12:11 UTC (permalink / raw)
To: gcc-patches; +Cc: rguenther, ook, richard.sandiford, nd
[-- Attachment #1: Type: text/plain, Size: 286 bytes --]
Hi all,
just a small patch removing a piece of unreachable code in
'vect_estimate_min_profitable_iters' given the condition
(LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)) is always true as
checked just above.
Bootstrapped on aarch64-unknown-linux-gnu.
Okay for trunk?
Andrea
[-- Attachment #2: 0001-vec-dead-code-removal-in-tree-vect-loop.c.patch --]
[-- Type: text/plain, Size: 1469 bytes --]
From 6bd96581410d9847ab78e6761178d4efbc9a5af2 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Fri, 4 Sep 2020 09:56:59 +0100
Subject: [PATCH] vec: dead code removal in tree-vect-loop.c
gcc/ChangeLog
2020-09-04 Andrea Corallo <andrea.corallo@arm.com>
* tree-vect-loop.c (vect_estimate_min_profitable_iters): Remove
dead code as LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo) is
always verified.
---
gcc/tree-vect-loop.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 362cdc4f1cb..38576382fe7 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -4101,17 +4101,10 @@ vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo,
if (outside_overhead > 0)
min_vec_niters = outside_overhead / saving_per_viter + 1;
- if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo))
- {
- int threshold = (vec_inside_cost * min_vec_niters
- + vec_outside_cost
- + scalar_outside_cost);
- min_profitable_estimate = threshold / scalar_single_iter_cost + 1;
- }
- else
- min_profitable_estimate = (min_vec_niters * assumed_vf
- + peel_iters_prologue
- + peel_iters_epilogue);
+ int threshold = (vec_inside_cost * min_vec_niters
+ + vec_outside_cost
+ + scalar_outside_cost);
+ min_profitable_estimate = threshold / scalar_single_iter_cost + 1;
}
else
{
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vec: remove unreachable code
2020-09-04 12:11 [PATCH] vec: remove unreachable code Andrea Corallo
@ 2020-09-04 12:12 ` Richard Biener
2020-09-04 12:20 ` Andrea Corallo
2020-09-04 15:37 ` Kewen.Lin
1 sibling, 1 reply; 8+ messages in thread
From: Richard Biener @ 2020-09-04 12:12 UTC (permalink / raw)
To: Andrea Corallo; +Cc: gcc-patches, ook, richard.sandiford, nd
On Fri, 4 Sep 2020, Andrea Corallo wrote:
> Hi all,
>
> just a small patch removing a piece of unreachable code in
> 'vect_estimate_min_profitable_iters' given the condition
> (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)) is always true as
> checked just above.
>
> Bootstrapped on aarch64-unknown-linux-gnu.
>
> Okay for trunk?
OK.
Richard.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vec: remove unreachable code
2020-09-04 12:12 ` Richard Biener
@ 2020-09-04 12:20 ` Andrea Corallo
0 siblings, 0 replies; 8+ messages in thread
From: Andrea Corallo @ 2020-09-04 12:20 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, ook, richard.sandiford, nd
Richard Biener <rguenther@suse.de> writes:
> On Fri, 4 Sep 2020, Andrea Corallo wrote:
>
>> Hi all,
>>
>> just a small patch removing a piece of unreachable code in
>> 'vect_estimate_min_profitable_iters' given the condition
>> (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)) is always true as
>> checked just above.
>>
>> Bootstrapped on aarch64-unknown-linux-gnu.
>>
>> Okay for trunk?
>
> OK.
>
> Richard.
1 min 4 secs, by far the quickest review I've ever got :)
Installed as 09fa6acd8d9
Thanks!
Andrea
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vec: remove unreachable code
2020-09-04 12:11 [PATCH] vec: remove unreachable code Andrea Corallo
2020-09-04 12:12 ` Richard Biener
@ 2020-09-04 15:37 ` Kewen.Lin
2020-09-07 11:16 ` Richard Sandiford
1 sibling, 1 reply; 8+ messages in thread
From: Kewen.Lin @ 2020-09-04 15:37 UTC (permalink / raw)
To: Andrea Corallo; +Cc: gcc-patches, nd, rguenther, ook, Richard Sandiford
Hi Andrea,
on 2020/9/4 下午8:11, Andrea Corallo wrote:
> Hi all,
>
> just a small patch removing a piece of unreachable code in
> 'vect_estimate_min_profitable_iters' given the condition
> (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)) is always true as
> checked just above.
>
FWIW, I had the same confusion when I saw the code at the first time,
the comments at outer if "??? The "if" arm ..." and the commit messages
seem to give some hints. IIUC, the code in this outer "if" was
implemented to handle all cases but that time was stage 4, so it's
conservative to be guarded as fully-predicated only to avoid the
potential risks. I was imagining that it would finally end up
by removing this outer if condition guard and the outer else.
But not sure why it didn't change in the following stage1.
Maybe Richard S. (the author) will have some comments.
BR,
Kewen
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vec: remove unreachable code
2020-09-04 15:37 ` Kewen.Lin
@ 2020-09-07 11:16 ` Richard Sandiford
2020-09-07 16:15 ` [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a comment Andrea Corallo
0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2020-09-07 11:16 UTC (permalink / raw)
To: Kewen.Lin; +Cc: Andrea Corallo, gcc-patches, nd, rguenther, ook
"Kewen.Lin" <linkw@linux.ibm.com> writes:
> Hi Andrea,
>
> on 2020/9/4 锟斤拷锟斤拷8:11, Andrea Corallo wrote:
>> Hi all,
>>
>> just a small patch removing a piece of unreachable code in
>> 'vect_estimate_min_profitable_iters' given the condition
>> (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)) is always true as
>> checked just above.
>>
>
> FWIW, I had the same confusion when I saw the code at the first time,
> the comments at outer if "??? The "if" arm ..." and the commit messages
> seem to give some hints. IIUC, the code in this outer "if" was
> implemented to handle all cases but that time was stage 4, so it's
> conservative to be guarded as fully-predicated only to avoid the
> potential risks. I was imagining that it would finally end up
> by removing this outer if condition guard and the outer else.
Right. So the idea was to show what the code would look like if we
removed the outer “if” condition. We would then always calculate the
estimate based on the minimum number of profitable vector iterations
first, then convert that into a minimum number of profitable scalar
iterations.
That still seems like a promising future direction. It would make the
estimate calculations more consistent and make it easier to compare the
cost of full-vector loops with the cost of partial-vector loops.
> But not sure why it didn't change in the following stage1.
Excess lameness on my part, sorry.
> Maybe Richard S. (the author) will have some comments.
TBH I'd prefer for the patch to be reverted. Maybe we could add
a version of the existing:
/* ??? The "if" arm is written to handle all cases; see below for what
we would do for !LOOP_VINFO_USING_PARTIAL_VECTORS_P. */
comment above:
else if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo))
{
E.g.:
/* ??? This "else if" arm is written to handle all cases; see below for
what we would do for !LOOP_VINFO_USING_PARTIAL_VECTORS_P. */
As:
/* This is a repeat of the code above, but with + SOC rather
than - SOC. */
says, this block is essentially repeating the logic from the earlier block,
so at the time it didn't seem like a good idea to duplicate the commentary.
Either way, the two blocks should be kept in sync, so if we want to remove
the path from this block, we should remove it from the earlier block too.
But my preference would be to keep the path in both blocks.
I'm not foolish enough to promise that I'll test/benchmark removing the
outer “if” condition in this stage 1 cycle, but I'll at least try to find
time…
Thanks,
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a comment.
2020-09-07 11:16 ` Richard Sandiford
@ 2020-09-07 16:15 ` Andrea Corallo
2020-09-07 17:20 ` Richard Sandiford
0 siblings, 1 reply; 8+ messages in thread
From: Andrea Corallo @ 2020-09-07 16:15 UTC (permalink / raw)
To: richard.sandiford; +Cc: gcc-patches, nd, rguenther, ook, Kewen.Lin
[-- Attachment #1: Type: text/plain, Size: 157 bytes --]
Hi Richard,
this reverts the discussed patch and adds what was suggested as a
comment.
Apologies for the inconvenience, okay for trunk?
Thanks
Andrea
[-- Attachment #2: 0001-vec-Revert-dead-code-removal-in-tree-vect-loop.c-and.patch --]
[-- Type: text/plain, Size: 1920 bytes --]
From 84dfa2d461692f445f45b3c3498f9bedba5c3848 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Mon, 7 Sep 2020 13:45:47 +0100
Subject: [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a
comment.
gcc/ChangeLog
2020-09-07 Andrea Corallo <andrea.corallo@arm.com>
* tree-vect-loop.c (vect_estimate_min_profitable_iters): Revert
dead-code removal introduced by 09fa6acd8d9 + add a comment to
clarify.
---
gcc/tree-vect-loop.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 9a82573604b..5ba4e594df9 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -4103,6 +4103,8 @@ vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo,
if (vec_outside_cost <= 0)
min_profitable_estimate = 0;
+ /* ??? This "else if" arm is written to handle all cases; see below for
+ what we would do for !LOOP_VINFO_USING_PARTIAL_VECTORS_P. */
else if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo))
{
/* This is a repeat of the code above, but with + SOC rather
@@ -4115,10 +4117,17 @@ vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo,
if (outside_overhead > 0)
min_vec_niters = outside_overhead / saving_per_viter + 1;
- int threshold = (vec_inside_cost * min_vec_niters
- + vec_outside_cost
- + scalar_outside_cost);
- min_profitable_estimate = threshold / scalar_single_iter_cost + 1;
+ if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo))
+ {
+ int threshold = (vec_inside_cost * min_vec_niters
+ + vec_outside_cost
+ + scalar_outside_cost);
+ min_profitable_estimate = threshold / scalar_single_iter_cost + 1;
+ }
+ else
+ min_profitable_estimate = (min_vec_niters * assumed_vf
+ + peel_iters_prologue
+ + peel_iters_epilogue);
}
else
{
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a comment.
2020-09-07 16:15 ` [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a comment Andrea Corallo
@ 2020-09-07 17:20 ` Richard Sandiford
2020-09-07 17:55 ` Andrea Corallo
0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2020-09-07 17:20 UTC (permalink / raw)
To: Andrea Corallo; +Cc: gcc-patches, nd, rguenther, ook, Kewen.Lin
Andrea Corallo <andrea.corallo@arm.com> writes:
> Hi Richard,
>
> this reverts the discussed patch and adds what was suggested as a
> comment.
>
> Apologies for the inconvenience, okay for trunk?
OK, thanks! And sorry for not commenting/documenting this very well.
Richard
>
> Thanks
>
> Andrea
>
> From 84dfa2d461692f445f45b3c3498f9bedba5c3848 Mon Sep 17 00:00:00 2001
> From: Andrea Corallo <andrea.corallo@arm.com>
> Date: Mon, 7 Sep 2020 13:45:47 +0100
> Subject: [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a
> comment.
>
> gcc/ChangeLog
>
> 2020-09-07 Andrea Corallo <andrea.corallo@arm.com>
>
> * tree-vect-loop.c (vect_estimate_min_profitable_iters): Revert
> dead-code removal introduced by 09fa6acd8d9 + add a comment to
> clarify.
> ---
> gcc/tree-vect-loop.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 9a82573604b..5ba4e594df9 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -4103,6 +4103,8 @@ vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo,
>
> if (vec_outside_cost <= 0)
> min_profitable_estimate = 0;
> + /* ??? This "else if" arm is written to handle all cases; see below for
> + what we would do for !LOOP_VINFO_USING_PARTIAL_VECTORS_P. */
> else if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo))
> {
> /* This is a repeat of the code above, but with + SOC rather
> @@ -4115,10 +4117,17 @@ vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo,
> if (outside_overhead > 0)
> min_vec_niters = outside_overhead / saving_per_viter + 1;
>
> - int threshold = (vec_inside_cost * min_vec_niters
> - + vec_outside_cost
> - + scalar_outside_cost);
> - min_profitable_estimate = threshold / scalar_single_iter_cost + 1;
> + if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo))
> + {
> + int threshold = (vec_inside_cost * min_vec_niters
> + + vec_outside_cost
> + + scalar_outside_cost);
> + min_profitable_estimate = threshold / scalar_single_iter_cost + 1;
> + }
> + else
> + min_profitable_estimate = (min_vec_niters * assumed_vf
> + + peel_iters_prologue
> + + peel_iters_epilogue);
> }
> else
> {
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a comment.
2020-09-07 17:20 ` Richard Sandiford
@ 2020-09-07 17:55 ` Andrea Corallo
0 siblings, 0 replies; 8+ messages in thread
From: Andrea Corallo @ 2020-09-07 17:55 UTC (permalink / raw)
To: richard.sandiford; +Cc: nd, rguenther, ook, Kewen.Lin, gcc-patches
Richard Sandiford <richard.sandiford@arm.com> writes:
> Andrea Corallo <andrea.corallo@arm.com> writes:
>> Hi Richard,
>>
>> this reverts the discussed patch and adds what was suggested as a
>> comment.
>>
>> Apologies for the inconvenience, okay for trunk?
>
> OK, thanks! And sorry for not commenting/documenting this very well.
>
> Richard
Installed as e147bb0faad.
Thanks
Andrea
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-07 17:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 12:11 [PATCH] vec: remove unreachable code Andrea Corallo
2020-09-04 12:12 ` Richard Biener
2020-09-04 12:20 ` Andrea Corallo
2020-09-04 15:37 ` Kewen.Lin
2020-09-07 11:16 ` Richard Sandiford
2020-09-07 16:15 ` [PATCH] vec: Revert "dead code removal in tree-vect-loop.c" and add a comment Andrea Corallo
2020-09-07 17:20 ` Richard Sandiford
2020-09-07 17:55 ` Andrea Corallo
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).