* [PATCH] Fix PR68306
@ 2015-11-12 12:39 Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-11-12 12:39 UTC (permalink / raw)
To: gcc-patches
The following fixes PR68306, an ordering issue with my last BB
vectorization patch. Fixed by removing that ordering requirement.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Richard.
2015-11-12 Richard Biener <rguenther@suse.de>
PR tree-optimization/68306
* tree-vect-data-refs.c (verify_data_ref_alignment): Remove
relevant and vectorizable checks here.
(vect_verify_datarefs_alignment): Add relevant check here.
* gcc.dg/pr68306.c: New testcase.
Index: gcc/tree-vect-data-refs.c
===================================================================
*** gcc/tree-vect-data-refs.c (revision 230216)
--- gcc/tree-vect-data-refs.c (working copy)
*************** verify_data_ref_alignment (data_referenc
*** 909,922 ****
gimple *stmt = DR_STMT (dr);
stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
! if (!STMT_VINFO_RELEVANT_P (stmt_info))
! return true;
!
! /* For interleaving, only the alignment of the first access matters.
! Skip statements marked as not vectorizable. */
! if ((STMT_VINFO_GROUPED_ACCESS (stmt_info)
! && GROUP_FIRST_ELEMENT (stmt_info) != stmt)
! || !STMT_VINFO_VECTORIZABLE (stmt_info))
return true;
/* Strided accesses perform only component accesses, alignment is
--- 889,897 ----
gimple *stmt = DR_STMT (dr);
stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
! /* For interleaving, only the alignment of the first access matters. */
! if (STMT_VINFO_GROUPED_ACCESS (stmt_info)
! && GROUP_FIRST_ELEMENT (stmt_info) != stmt)
return true;
/* Strided accesses perform only component accesses, alignment is
*************** vect_verify_datarefs_alignment (loop_vec
*** 965,972 ****
unsigned int i;
FOR_EACH_VEC_ELT (datarefs, i, dr)
! if (! verify_data_ref_alignment (dr))
! return false;
return true;
}
--- 940,954 ----
unsigned int i;
FOR_EACH_VEC_ELT (datarefs, i, dr)
! {
! gimple *stmt = DR_STMT (dr);
! stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
!
! if (!STMT_VINFO_RELEVANT_P (stmt_info))
! continue;
! if (! verify_data_ref_alignment (dr))
! return false;
! }
return true;
}
Index: gcc/testsuite/gcc.dg/pr68306.c
===================================================================
*** gcc/testsuite/gcc.dg/pr68306.c (revision 0)
--- gcc/testsuite/gcc.dg/pr68306.c (working copy)
***************
*** 0 ****
--- 1,10 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O3" } */
+
+ enum powerpc_pmc_type { PPC_PMC_IBM };
+ struct {
+ unsigned num_pmcs;
+ enum powerpc_pmc_type pmc_type;
+ } a;
+ enum powerpc_pmc_type b;
+ void fn1() { a.num_pmcs = a.pmc_type = b; }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR68306
2015-11-13 13:15 Uros Bizjak
@ 2015-11-13 18:41 ` Uros Bizjak
0 siblings, 0 replies; 4+ messages in thread
From: Uros Bizjak @ 2015-11-13 18:41 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener
[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]
On Fri, Nov 13, 2015 at 2:15 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> 2015-11-13 Richard Biener <rguenther@suse.de>
>>
>> PR tree-optimization/68306
>> * tree-vect-data-refs.c (verify_data_ref_alignment): Move
>> loop related checks ...
>> (vect_verify_datarefs_alignment): ... here.
>> (vect_slp_analyze_and_verify_node_alignment): Compute and
>> verify alignment of the single DR that it matters.
>> * tree-vect-stmts.c (vectorizable_store): Add an assert.
>> (vectorizable_load): Add a comment.
>> * tree-vect-slp.c (vect_analyze_slp_cost_1): Fix DR used
>> for determining load cost.
>>
>> * gcc.dg/pr68306.c: Adjust.
>> * gcc.dg/pr68306-2.c: New testcase.
>> * gcc.dg/pr68306-3.c: Likewise.
>
> + /* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
>
> You should use { target i?86-*-* x86_64-*-* } here and in a couple of
> other places.
Added by attached patch.
2015-11-13 Uros Bizjak <ubizjak@gmail.com>
* gcc.dg/pr68306.c (dg-additional-options): Add i?86-*-* target.
* gcc.dg/pr68306-2.c (dg-additional-options): Ditto.
* gcc.dg/pr68306-3.c (dg-additional-options): Ditto.
Tested on x86_64-linux-gnu {,-m32} and committed to mainline SVN.
Uros.
[-- Attachment #2: t.diff.txt --]
[-- Type: text/plain, Size: 1878 bytes --]
Index: ChangeLog
===================================================================
--- ChangeLog (revision 230338)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2015-11-13 Uros Bizjak <ubizjak@gmail.com>
+
+ * gcc.dg/pr68306.c (dg-additional-options): Add i?86-*-* target.
+ * gcc.dg/pr68306-2.c (dg-additional-options): Ditto.
+ * gcc.dg/pr68306-3.c (dg-additional-options): Ditto.
+
2015-11-13 David Malcolm <dmalcolm@redhat.com>
* gcc.dg/diagnostic-token-ranges.c: New file.
Index: gcc.dg/pr68306-2.c
===================================================================
--- gcc.dg/pr68306-2.c (revision 230338)
+++ gcc.dg/pr68306-2.c (working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O3" } */
-/* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
+/* { dg-additional-options "-mno-sse -mno-mmx" { target i?86-*-* x86_64-*-* } } */
struct {
int tz_minuteswest;
Index: gcc.dg/pr68306-3.c
===================================================================
--- gcc.dg/pr68306-3.c (revision 230338)
+++ gcc.dg/pr68306-3.c (working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O3" } */
-/* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
+/* { dg-additional-options "-mno-sse -mno-mmx" { target i?86-*-* x86_64-*-* } } */
/* { dg-additional-options "-mno-altivec -mno-vsx" { target powerpc*-*-* } } */
extern void fn2();
Index: gcc.dg/pr68306.c
===================================================================
--- gcc.dg/pr68306.c (revision 230338)
+++ gcc.dg/pr68306.c (working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O3" } */
-/* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
+/* { dg-additional-options "-mno-sse -mno-mmx" { target i?86-*-* x86_64-*-* } } */
enum powerpc_pmc_type { PPC_PMC_IBM };
struct {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR68306
@ 2015-11-13 13:15 Uros Bizjak
2015-11-13 18:41 ` Uros Bizjak
0 siblings, 1 reply; 4+ messages in thread
From: Uros Bizjak @ 2015-11-13 13:15 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener
Hello!
> 2015-11-13 Richard Biener <rguenther@suse.de>
>
> PR tree-optimization/68306
> * tree-vect-data-refs.c (verify_data_ref_alignment): Move
> loop related checks ...
> (vect_verify_datarefs_alignment): ... here.
> (vect_slp_analyze_and_verify_node_alignment): Compute and
> verify alignment of the single DR that it matters.
> * tree-vect-stmts.c (vectorizable_store): Add an assert.
> (vectorizable_load): Add a comment.
> * tree-vect-slp.c (vect_analyze_slp_cost_1): Fix DR used
> for determining load cost.
>
> * gcc.dg/pr68306.c: Adjust.
> * gcc.dg/pr68306-2.c: New testcase.
> * gcc.dg/pr68306-3.c: Likewise.
+ /* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
You should use { target i?86-*-* x86_64-*-* } here and in a couple of
other places.
Uros.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Fix PR68306
@ 2015-11-13 12:01 Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-11-13 12:01 UTC (permalink / raw)
To: gcc-patches
This fixes more of PR68306. Digging into the details reveals
that we only need to check a single (but the correct one) DR
and that cost modeling also gets this wrong.
Thus fixed.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
Richard.
2015-11-13 Richard Biener <rguenther@suse.de>
PR tree-optimization/68306
* tree-vect-data-refs.c (verify_data_ref_alignment): Move
loop related checks ...
(vect_verify_datarefs_alignment): ... here.
(vect_slp_analyze_and_verify_node_alignment): Compute and
verify alignment of the single DR that it matters.
* tree-vect-stmts.c (vectorizable_store): Add an assert.
(vectorizable_load): Add a comment.
* tree-vect-slp.c (vect_analyze_slp_cost_1): Fix DR used
for determining load cost.
* gcc.dg/pr68306.c: Adjust.
* gcc.dg/pr68306-2.c: New testcase.
* gcc.dg/pr68306-3.c: Likewise.
Index: gcc/tree-vect-data-refs.c
===================================================================
*** gcc/tree-vect-data-refs.c (revision 230293)
--- gcc/tree-vect-data-refs.c (working copy)
*************** verify_data_ref_alignment (data_referenc
*** 920,936 ****
gimple *stmt = DR_STMT (dr);
stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
- /* For interleaving, only the alignment of the first access matters. */
- if (STMT_VINFO_GROUPED_ACCESS (stmt_info)
- && GROUP_FIRST_ELEMENT (stmt_info) != stmt)
- return true;
-
- /* Strided accesses perform only component accesses, alignment is
- irrelevant for them. */
- if (STMT_VINFO_STRIDED_P (stmt_info)
- && !STMT_VINFO_GROUPED_ACCESS (stmt_info))
- return true;
-
supportable_dr_alignment = vect_supportable_dr_alignment (dr, false);
if (!supportable_dr_alignment)
{
--- 920,925 ----
*************** vect_verify_datarefs_alignment (loop_vec
*** 977,982 ****
--- 966,983 ----
if (!STMT_VINFO_RELEVANT_P (stmt_info))
continue;
+
+ /* For interleaving, only the alignment of the first access matters. */
+ if (STMT_VINFO_GROUPED_ACCESS (stmt_info)
+ && GROUP_FIRST_ELEMENT (stmt_info) != stmt)
+ return true;
+
+ /* Strided accesses perform only component accesses, alignment is
+ irrelevant for them. */
+ if (STMT_VINFO_STRIDED_P (stmt_info)
+ && !STMT_VINFO_GROUPED_ACCESS (stmt_info))
+ return true;
+
if (! verify_data_ref_alignment (dr))
return false;
}
*************** vect_analyze_data_refs_alignment (loop_v
*** 2100,2127 ****
static bool
vect_slp_analyze_and_verify_node_alignment (slp_tree node)
{
! unsigned i;
! gimple *stmt;
! FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)
{
! stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
!
! /* Strided accesses perform only component accesses, misalignment
! information is irrelevant for them. */
! if (STMT_VINFO_STRIDED_P (stmt_info)
! && !STMT_VINFO_GROUPED_ACCESS (stmt_info))
! continue;
!
! data_reference_p dr = STMT_VINFO_DATA_REF (stmt_info);
! if (! vect_compute_data_ref_alignment (dr)
! || ! verify_data_ref_alignment (dr))
! {
! if (dump_enabled_p ())
! dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
! "not vectorized: bad data alignment in basic "
! "block.\n");
! return false;
! }
}
return true;
--- 2101,2122 ----
static bool
vect_slp_analyze_and_verify_node_alignment (slp_tree node)
{
! /* We vectorize from the first scalar stmt in the node unless
! the node is permuted in which case we start from the first
! element in the group. */
! gimple *first_stmt = SLP_TREE_SCALAR_STMTS (node)[0];
! if (SLP_TREE_LOAD_PERMUTATION (node).exists ())
! first_stmt = GROUP_FIRST_ELEMENT (vinfo_for_stmt (first_stmt));
!
! data_reference_p dr = STMT_VINFO_DATA_REF (vinfo_for_stmt (first_stmt));
! if (! vect_compute_data_ref_alignment (dr)
! || ! verify_data_ref_alignment (dr))
{
! if (dump_enabled_p ())
! dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
! "not vectorized: bad data alignment in basic "
! "block.\n");
! return false;
}
return true;
Index: gcc/tree-vect-stmts.c
===================================================================
*** gcc/tree-vect-stmts.c (revision 230293)
--- gcc/tree-vect-stmts.c (working copy)
*************** vectorizable_store (gimple *stmt, gimple
*** 5464,5469 ****
--- 5464,5470 ----
group. */
vec_num = SLP_TREE_NUMBER_OF_VEC_STMTS (slp_node);
first_stmt = SLP_TREE_SCALAR_STMTS (slp_node)[0];
+ gcc_assert (GROUP_FIRST_ELEMENT (vinfo_for_stmt (first_stmt)) == first_stmt);
first_dr = STMT_VINFO_DATA_REF (vinfo_for_stmt (first_stmt));
op = gimple_assign_rhs1 (first_stmt);
}
*************** vectorizable_load (gimple *stmt, gimple_
*** 6658,6666 ****
if (grouped_load)
{
first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
! if (slp
! && !SLP_TREE_LOAD_PERMUTATION (slp_node).exists ()
! && first_stmt != SLP_TREE_SCALAR_STMTS (slp_node)[0])
first_stmt = SLP_TREE_SCALAR_STMTS (slp_node)[0];
/* Check if the chain of loads is already vectorized. */
--- 6659,6667 ----
if (grouped_load)
{
first_stmt = GROUP_FIRST_ELEMENT (stmt_info);
! /* For BB vectorization we directly vectorize a subchain
! without permutation. */
! if (slp && ! SLP_TREE_LOAD_PERMUTATION (slp_node).exists ())
first_stmt = SLP_TREE_SCALAR_STMTS (slp_node)[0];
/* Check if the chain of loads is already vectorized. */
Index: gcc/tree-vect-slp.c
===================================================================
*** gcc/tree-vect-slp.c (revision 230293)
--- gcc/tree-vect-slp.c (working copy)
*************** vect_analyze_slp_cost_1 (slp_instance in
*** 1429,1434 ****
--- 1429,1441 ----
{
int i;
gcc_checking_assert (DR_IS_READ (STMT_VINFO_DATA_REF (stmt_info)));
+ /* If the load is permuted then the alignment is determined by
+ the first group element not by the first scalar stmt DR. */
+ if (SLP_TREE_LOAD_PERMUTATION (node).exists ())
+ {
+ stmt = GROUP_FIRST_ELEMENT (stmt_info);
+ stmt_info = vinfo_for_stmt (stmt);
+ }
vect_model_load_cost (stmt_info, ncopies_for_cost, false,
node, prologue_cost_vec, body_cost_vec);
/* If the load is permuted record the cost for the permutation.
Index: gcc/testsuite/gcc.dg/pr68306.c
===================================================================
*** gcc/testsuite/gcc.dg/pr68306.c (revision 230293)
--- gcc/testsuite/gcc.dg/pr68306.c (working copy)
***************
*** 1,5 ****
--- 1,6 ----
/* { dg-do compile } */
/* { dg-options "-O3" } */
+ /* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
enum powerpc_pmc_type { PPC_PMC_IBM };
struct {
Index: gcc/testsuite/gcc.dg/pr68306-2.c
===================================================================
*** gcc/testsuite/gcc.dg/pr68306-2.c (revision 0)
--- gcc/testsuite/gcc.dg/pr68306-2.c (revision 0)
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O3" } */
+ /* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
+
+ struct {
+ int tz_minuteswest;
+ int tz_dsttime;
+ } a, b;
+ void fn1() {
+ b.tz_minuteswest = a.tz_minuteswest;
+ b.tz_dsttime = a.tz_dsttime;
+ }
Index: gcc/testsuite/gcc.dg/pr68306-3.c
===================================================================
*** gcc/testsuite/gcc.dg/pr68306-3.c (revision 0)
--- gcc/testsuite/gcc.dg/pr68306-3.c (revision 0)
***************
*** 0 ****
--- 1,21 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O3" } */
+ /* { dg-additional-options "-mno-sse -mno-mmx" { target x86_64-*-* } } */
+ /* { dg-additional-options "-mno-altivec -mno-vsx" { target powerpc*-*-* } } */
+
+ extern void fn2();
+ struct {
+ unsigned qp_num;
+ unsigned starting_psn;
+ void *private_data;
+ } a;
+ struct {
+ unsigned id;
+ unsigned qpn;
+ unsigned psn;
+ } b;
+ void fn1() {
+ a.qp_num = b.qpn;
+ a.starting_psn = b.psn;
+ fn2(b.id);
+ }
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-13 18:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-12 12:39 [PATCH] Fix PR68306 Richard Biener
2015-11-13 12:01 Richard Biener
2015-11-13 13:15 Uros Bizjak
2015-11-13 18:41 ` Uros Bizjak
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).