* [PATCH] Fix PR94043 by making vect_live_op generate lc-phi
@ 2020-03-30 10:24 Kewen.Lin
2020-03-30 10:38 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Kewen.Lin @ 2020-03-30 10:24 UTC (permalink / raw)
To: GCC Patches; +Cc: Segher Boessenkool, Bill Schmidt, Richard Biener
[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]
Hi,
As PR94043 shows, my commit r10-4524 exposed one issue in
vectorizable_live_operation, which inserts one extra BB
before the single exit, leading unexpected operand expansion
and unexpected loop depth assertion. As Richi suggested,
this patch is to teach vectorizable_live_operation to
generate loop closed phi for vec_lhs, it looks like:
loop;
# lhs' = PHI <lhs>
=>
loop;
# vec_lhs' = PHI <vec_lhs>
new_tree = BIT_FIELD_REF <vec_lhs', ...>;
lhs' = new_tree;
I noticed that there are some SLP cases that have same lhs
and vec_lhs but different offsets, which can make us have
more PHIs for the same vec_lhs there. But I think it would
be fine since only one of them is actually live, the others
should be eliminated by the following dce. So the patch
doesn't check whether there is one phi for vec_lhs, just
create one directly instead.
Bootstrapped/regtested on powerpc64le-linux-gnu (LE) P8.
Is it ok for trunk?
BR,
Kewen
-----------
gcc/ChangeLog
2020-MM-DD Kewen Lin <linkw@gcc.gnu.org>
PR tree-optimization/94043
* tree-vect-loop.c (vectorizable_live_operation): Generate loop-closed
phi for vec_lhs and use it for lane extraction.
gcc/testsuite/ChangeLog
2020-MM-DD Kewen Lin <linkw@gcc.gnu.org>
PR tree-optimization/94043
* gfortran.dg/graphite/vect-pr94043.f90: New test.
[-- Attachment #2: vect_live_lcphi.diff --]
[-- Type: text/plain, Size: 3953 bytes --]
diff --git a/gcc/testsuite/gfortran.dg/graphite/vect-pr94043.f90 b/gcc/testsuite/gfortran.dg/graphite/vect-pr94043.f90
new file mode 100644
index 00000000000..744c0f3042e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/graphite/vect-pr94043.f90
@@ -0,0 +1,18 @@
+! { dg-do compile }
+! { dg-additional-options "-O3 -ftree-parallelize-loops=2 -fno-tree-dce" }
+
+! As PR94043, test it to be compiled successfully without ICE.
+
+program yw
+ integer :: hx(6, 6)
+ integer :: ps = 1, e2 = 1
+
+ do ps = 1, 6
+ do e2 = 1, 6
+ hx(e2, ps) = 0
+ if (ps >= 5 .and. e2 >= 5) then
+ hx(e2, ps) = hx(1, 1)
+ end if
+ end do
+ end do
+end program
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 53fccb715ef..eb607f3aab0 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -7998,6 +7998,25 @@ vectorizable_live_operation (stmt_vec_info stmt_info,
bitstart = int_const_binop (MINUS_EXPR, vec_bitsize, bitsize);
}
+ /* Ensure the VEC_LHS for lane extraction stmts satisfy loop-closed PHI
+ requirement, insert one phi node for it. It looks like:
+ loop;
+ BB:
+ # lhs' = PHI <lhs>
+ ==>
+ loop;
+ BB:
+ # vec_lhs' = PHI <vec_lhs>
+ new_tree = lane_extract <vec_lhs', ...>;
+ lhs' = new_tree; */
+
+ basic_block exit_bb = single_exit (loop)->dest;
+ gcc_assert (single_pred_p (exit_bb));
+
+ tree vec_lhs_phi = copy_ssa_name (vec_lhs);
+ gimple *phi = create_phi_node (vec_lhs_phi, exit_bb);
+ SET_PHI_ARG_DEF (phi, single_exit (loop)->dest_idx, vec_lhs);
+
gimple_seq stmts = NULL;
tree new_tree;
if (LOOP_VINFO_FULLY_MASKED_P (loop_vinfo))
@@ -8010,10 +8029,10 @@ vectorizable_live_operation (stmt_vec_info stmt_info,
the loop mask for the final iteration. */
gcc_assert (ncopies == 1 && !slp_node);
tree scalar_type = TREE_TYPE (STMT_VINFO_VECTYPE (stmt_info));
- tree mask = vect_get_loop_mask (gsi, &LOOP_VINFO_MASKS (loop_vinfo),
- 1, vectype, 0);
- tree scalar_res = gimple_build (&stmts, CFN_EXTRACT_LAST,
- scalar_type, mask, vec_lhs);
+ tree mask = vect_get_loop_mask (gsi, &LOOP_VINFO_MASKS (loop_vinfo), 1,
+ vectype, 0);
+ tree scalar_res = gimple_build (&stmts, CFN_EXTRACT_LAST, scalar_type,
+ mask, vec_lhs_phi);
/* Convert the extracted vector element to the required scalar type. */
new_tree = gimple_convert (&stmts, lhs_type, scalar_res);
@@ -8023,13 +8042,32 @@ vectorizable_live_operation (stmt_vec_info stmt_info,
tree bftype = TREE_TYPE (vectype);
if (VECTOR_BOOLEAN_TYPE_P (vectype))
bftype = build_nonstandard_integer_type (tree_to_uhwi (bitsize), 1);
- new_tree = build3 (BIT_FIELD_REF, bftype, vec_lhs, bitsize, bitstart);
+ new_tree = build3 (BIT_FIELD_REF, bftype, vec_lhs_phi, bitsize, bitstart);
new_tree = force_gimple_operand (fold_convert (lhs_type, new_tree),
&stmts, true, NULL_TREE);
}
if (stmts)
- gsi_insert_seq_on_edge_immediate (single_exit (loop), stmts);
+ {
+ gimple_stmt_iterator exit_gsi = gsi_after_labels (exit_bb);
+ gsi_insert_before (&exit_gsi, stmts, GSI_CONTINUE_LINKING);
+
+ /* Remove existing phi from lhs and create one copy from new_tree. */
+ tree lhs_phi = NULL_TREE;
+ gimple_stmt_iterator gsi;
+ for (gsi = gsi_start_phis (exit_bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *phi = gsi_stmt (gsi);
+ if ((gimple_phi_arg_def (phi, 0) == lhs))
+ {
+ remove_phi_node (&gsi, false);
+ lhs_phi = gimple_phi_result (phi);
+ gimple *copy = gimple_build_assign (lhs_phi, new_tree);
+ gsi_insert_after (&exit_gsi, copy, GSI_CONTINUE_LINKING);
+ break;
+ }
+ }
+ }
/* Replace use of lhs with newly computed result. If the use stmt is a
single arg PHI, just replace all uses of PHI result. It's necessary
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR94043 by making vect_live_op generate lc-phi
2020-03-30 10:24 [PATCH] Fix PR94043 by making vect_live_op generate lc-phi Kewen.Lin
@ 2020-03-30 10:38 ` Richard Biener
2020-04-01 22:51 ` H.J. Lu
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2020-03-30 10:38 UTC (permalink / raw)
To: Kewen.Lin; +Cc: GCC Patches, Segher Boessenkool, Bill Schmidt
On Mon, Mar 30, 2020 at 12:24 PM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> Hi,
>
> As PR94043 shows, my commit r10-4524 exposed one issue in
> vectorizable_live_operation, which inserts one extra BB
> before the single exit, leading unexpected operand expansion
> and unexpected loop depth assertion. As Richi suggested,
> this patch is to teach vectorizable_live_operation to
> generate loop closed phi for vec_lhs, it looks like:
> loop;
> # lhs' = PHI <lhs>
> =>
> loop;
> # vec_lhs' = PHI <vec_lhs>
> new_tree = BIT_FIELD_REF <vec_lhs', ...>;
> lhs' = new_tree;
>
> I noticed that there are some SLP cases that have same lhs
> and vec_lhs but different offsets, which can make us have
> more PHIs for the same vec_lhs there. But I think it would
> be fine since only one of them is actually live, the others
> should be eliminated by the following dce. So the patch
> doesn't check whether there is one phi for vec_lhs, just
> create one directly instead.
>
> Bootstrapped/regtested on powerpc64le-linux-gnu (LE) P8.
>
> Is it ok for trunk?
OK.
Thanks,
Richard.
> BR,
> Kewen
> -----------
>
> gcc/ChangeLog
>
> 2020-MM-DD Kewen Lin <linkw@gcc.gnu.org>
>
> PR tree-optimization/94043
> * tree-vect-loop.c (vectorizable_live_operation): Generate loop-closed
> phi for vec_lhs and use it for lane extraction.
>
> gcc/testsuite/ChangeLog
>
> 2020-MM-DD Kewen Lin <linkw@gcc.gnu.org>
>
> PR tree-optimization/94043
> * gfortran.dg/graphite/vect-pr94043.f90: New test.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR94043 by making vect_live_op generate lc-phi
2020-03-30 10:38 ` Richard Biener
@ 2020-04-01 22:51 ` H.J. Lu
2020-04-02 10:43 ` [PATCH] Fix PR94443 with gsi_insert_seq_before Kewen.Lin
0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2020-04-01 22:51 UTC (permalink / raw)
To: Richard Biener; +Cc: Kewen.Lin, Bill Schmidt, GCC Patches, Segher Boessenkool
On Mon, Mar 30, 2020 at 4:09 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Mon, Mar 30, 2020 at 12:24 PM Kewen.Lin <linkw@linux.ibm.com> wrote:
> >
> > Hi,
> >
> > As PR94043 shows, my commit r10-4524 exposed one issue in
> > vectorizable_live_operation, which inserts one extra BB
> > before the single exit, leading unexpected operand expansion
> > and unexpected loop depth assertion. As Richi suggested,
> > this patch is to teach vectorizable_live_operation to
> > generate loop closed phi for vec_lhs, it looks like:
> > loop;
> > # lhs' = PHI <lhs>
> > =>
> > loop;
> > # vec_lhs' = PHI <vec_lhs>
> > new_tree = BIT_FIELD_REF <vec_lhs', ...>;
> > lhs' = new_tree;
> >
> > I noticed that there are some SLP cases that have same lhs
> > and vec_lhs but different offsets, which can make us have
> > more PHIs for the same vec_lhs there. But I think it would
> > be fine since only one of them is actually live, the others
> > should be eliminated by the following dce. So the patch
> > doesn't check whether there is one phi for vec_lhs, just
> > create one directly instead.
> >
> > Bootstrapped/regtested on powerpc64le-linux-gnu (LE) P8.
> >
> > Is it ok for trunk?
>
> OK.
>
> Thanks,
> Richard.
>
> > BR,
> > Kewen
> > -----------
> >
> > gcc/ChangeLog
> >
> > 2020-MM-DD Kewen Lin <linkw@gcc.gnu.org>
> >
> > PR tree-optimization/94043
> > * tree-vect-loop.c (vectorizable_live_operation): Generate loop-closed
> > phi for vec_lhs and use it for lane extraction.
> >
> > gcc/testsuite/ChangeLog
> >
> > 2020-MM-DD Kewen Lin <linkw@gcc.gnu.org>
> >
> > PR tree-optimization/94043
> > * gfortran.dg/graphite/vect-pr94043.f90: New test.
> >
This caused:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94449
--
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Fix PR94443 with gsi_insert_seq_before
2020-04-01 22:51 ` H.J. Lu
@ 2020-04-02 10:43 ` Kewen.Lin
2020-04-02 18:55 ` H.J. Lu
2020-04-03 7:01 ` Richard Biener
0 siblings, 2 replies; 6+ messages in thread
From: Kewen.Lin @ 2020-04-02 10:43 UTC (permalink / raw)
To: GCC Patches; +Cc: H.J. Lu, Richard Biener, Bill Schmidt, Segher Boessenkool
[-- Attachment #1: Type: text/plain, Size: 936 bytes --]
on 2020/4/2 上午6:51, H.J. Lu wrote:
>
> This caused:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94449
>
Thanks for reporting this. The attached patch is to fix the stupid
mistake by using gsi_insert_seq_before instead of gsi_insert_before.
BTW, the regression testing on one x86_64 machine from CFarm is
unable to reveal it (I guess due to native arch sandybridge?), so I
specified additional option -march=znver2 and verified the coverage.
Bootstrapped/regtested on powerpc64le-linux-gnu (P9) and
x86_64-pc-linux-gnu, also verified the fail cases in related PRs.
BR,
Kewen
-----------
gcc/ChangeLog
2020-04-02 Kewen Lin <linkw@gcc.gnu.org>
PR tree-optimization/94443
* tree-vect-loop.c (vectorizable_live_operation): Use
gsi_insert_seq_before to replace gsi_insert_before.
gcc/testsuite/ChangeLog
2020-04-02 Kewen Lin <linkw@gcc.gnu.org>
PR tree-optimization/94443
* gcc.dg/vect/pr94443.c: New test.
[-- Attachment #2: PR94443.diff --]
[-- Type: text/plain, Size: 1391 bytes --]
diff --git a/gcc/testsuite/gcc.dg/vect/pr94443.c b/gcc/testsuite/gcc.dg/vect/pr94443.c
new file mode 100644
index 0000000..f8cbaf1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr94443.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=znver2" { target { x86_64-*-* i?86-*-* } } } */
+
+/* Check it to be compiled successfully without any ICE. */
+
+int a;
+unsigned *b;
+
+void foo()
+{
+ for (unsigned i; i <= a; ++i, ++b)
+ ;
+}
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index c9b6534..34adf79 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -8050,7 +8050,7 @@ vectorizable_live_operation (stmt_vec_info stmt_info,
if (stmts)
{
gimple_stmt_iterator exit_gsi = gsi_after_labels (exit_bb);
- gsi_insert_before (&exit_gsi, stmts, GSI_CONTINUE_LINKING);
+ gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
/* Remove existing phi from lhs and create one copy from new_tree. */
tree lhs_phi = NULL_TREE;
@@ -8063,7 +8063,7 @@ vectorizable_live_operation (stmt_vec_info stmt_info,
remove_phi_node (&gsi, false);
lhs_phi = gimple_phi_result (phi);
gimple *copy = gimple_build_assign (lhs_phi, new_tree);
- gsi_insert_after (&exit_gsi, copy, GSI_CONTINUE_LINKING);
+ gsi_insert_before (&exit_gsi, copy, GSI_SAME_STMT);
break;
}
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR94443 with gsi_insert_seq_before
2020-04-02 10:43 ` [PATCH] Fix PR94443 with gsi_insert_seq_before Kewen.Lin
@ 2020-04-02 18:55 ` H.J. Lu
2020-04-03 7:01 ` Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2020-04-02 18:55 UTC (permalink / raw)
To: Kewen.Lin; +Cc: GCC Patches, Richard Biener, Bill Schmidt, Segher Boessenkool
On Thu, Apr 2, 2020 at 3:43 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> on 2020/4/2 上午6:51, H.J. Lu wrote:
> >
> > This caused:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94449
> >
>
> Thanks for reporting this. The attached patch is to fix the stupid
> mistake by using gsi_insert_seq_before instead of gsi_insert_before.
>
> BTW, the regression testing on one x86_64 machine from CFarm is
> unable to reveal it (I guess due to native arch sandybridge?), so I
> specified additional option -march=znver2 and verified the coverage.
>
> Bootstrapped/regtested on powerpc64le-linux-gnu (P9) and
> x86_64-pc-linux-gnu, also verified the fail cases in related PRs.
>
>
> BR,
> Kewen
> -----------
> gcc/ChangeLog
>
> 2020-04-02 Kewen Lin <linkw@gcc.gnu.org>
>
> PR tree-optimization/94443
> * tree-vect-loop.c (vectorizable_live_operation): Use
> gsi_insert_seq_before to replace gsi_insert_before.
>
> gcc/testsuite/ChangeLog
>
> 2020-04-02 Kewen Lin <linkw@gcc.gnu.org>
>
> PR tree-optimization/94443
> * gcc.dg/vect/pr94443.c: New test.
>
I verified that this fixed my bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94449
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix PR94443 with gsi_insert_seq_before
2020-04-02 10:43 ` [PATCH] Fix PR94443 with gsi_insert_seq_before Kewen.Lin
2020-04-02 18:55 ` H.J. Lu
@ 2020-04-03 7:01 ` Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: Richard Biener @ 2020-04-03 7:01 UTC (permalink / raw)
To: Kewen.Lin; +Cc: GCC Patches, H.J. Lu, Bill Schmidt, Segher Boessenkool
On Thu, Apr 2, 2020 at 12:43 PM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> on 2020/4/2 上午6:51, H.J. Lu wrote:
> >
> > This caused:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94449
> >
>
> Thanks for reporting this. The attached patch is to fix the stupid
> mistake by using gsi_insert_seq_before instead of gsi_insert_before.
>
> BTW, the regression testing on one x86_64 machine from CFarm is
> unable to reveal it (I guess due to native arch sandybridge?), so I
> specified additional option -march=znver2 and verified the coverage.
>
> Bootstrapped/regtested on powerpc64le-linux-gnu (P9) and
> x86_64-pc-linux-gnu, also verified the fail cases in related PRs.
OK.
Richard.
>
> BR,
> Kewen
> -----------
> gcc/ChangeLog
>
> 2020-04-02 Kewen Lin <linkw@gcc.gnu.org>
>
> PR tree-optimization/94443
> * tree-vect-loop.c (vectorizable_live_operation): Use
> gsi_insert_seq_before to replace gsi_insert_before.
>
> gcc/testsuite/ChangeLog
>
> 2020-04-02 Kewen Lin <linkw@gcc.gnu.org>
>
> PR tree-optimization/94443
> * gcc.dg/vect/pr94443.c: New test.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-03 7:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 10:24 [PATCH] Fix PR94043 by making vect_live_op generate lc-phi Kewen.Lin
2020-03-30 10:38 ` Richard Biener
2020-04-01 22:51 ` H.J. Lu
2020-04-02 10:43 ` [PATCH] Fix PR94443 with gsi_insert_seq_before Kewen.Lin
2020-04-02 18:55 ` H.J. Lu
2020-04-03 7:01 ` 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).