From: Thomas Schwinge <thomas@codesourcery.com>
To: Richard Biener <rguenther@suse.de>,
Andrew Stubbs <ams@codesourcery.com>,
Julian Brown <julian@codesourcery.com>
Cc: <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] tree-optimization/113073 - amend PR112736 fix
Date: Wed, 20 Dec 2023 10:50:07 +0100 [thread overview]
Message-ID: <871qbh4300.fsf@euler.schwinge.homeip.net> (raw)
In-Reply-To: <20231219123224.3D481385E459@sourceware.org>
Hi!
On 2023-12-19T13:30:58+0100, Richard Biener <rguenther@suse.de> wrote:
> The PR112736 testcase fails on RISC-V because the aligned exception
> uses the wrong check. The alignment support scheme can be
> dr_aligned even when the access isn't aligned to the vector size
> but some targets are happy with element alignment. The following
> fixes that.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
I've noticed this to regresses GCN target as follows:
PASS: gcc.dg/vect/bb-slp-pr78205.c (test for excess errors)
PASS: gcc.dg/vect/bb-slp-pr78205.c scan-tree-dump-times slp2 "optimized: basic block" 3
PASS: gcc.dg/vect/bb-slp-pr78205.c scan-tree-dump-times slp2 "BB vectorization with gaps at the end of a load is not supported" 1
[-PASS:-]{+FAIL:+} gcc.dg/vect/bb-slp-pr78205.c scan-tree-dump-times optimized " = c\\[4\\];" 1
As so often, I've got no clue whether that's a vectorizer, GCN back end,
or test case issue. ;-)
'diff'ing before vs. after:
--- bb-slp-pr78205.c.191t.slp2 2023-12-20 09:49:45.834344620 +0100
+++ bb-slp-pr78205.c.191t.slp2 2023-12-20 09:10:14.706300941 +0100
[...]
@@ -505,8 +505,9 @@
[...]/bb-slp-pr78205.c:9:8: note: create vector_type-pointer variable to type: vector(4) double vectorizing a pointer ref: c[0]
[...]/bb-slp-pr78205.c:9:8: note: created &c[0]
[...]/bb-slp-pr78205.c:9:8: note: add new stmt: vect__1.7_19 = MEM <vector(4) double> [(double *)&c];
-[...]/bb-slp-pr78205.c:9:8: note: add new stmt: vect__1.8_20 = MEM <vector(4) double> [(double *)&c + 32B];
-[...]/bb-slp-pr78205.c:9:8: note: add new stmt: vect__1.9_21 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 0, 1, 0, 1 }>;
+[...]/bb-slp-pr78205.c:9:8: note: add new stmt: _20 = MEM[(double *)&c + 32B];
+[...]/bb-slp-pr78205.c:9:8: note: add new stmt: vect__1.8_21 = {_20, 0.0, 0.0, 0.0};
+[...]/bb-slp-pr78205.c:9:8: note: add new stmt: vect__1.9_22 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 0, 1, 0, 1 }>;
[...]/bb-slp-pr78205.c:9:8: note: ------>vectorizing SLP node starting from: a[0] = _1;
[...]/bb-slp-pr78205.c:9:8: note: vect_is_simple_use: operand c[0], type of def: internal
[...]/bb-slp-pr78205.c:9:8: note: vect_is_simple_use: operand c[1], type of def: internal
[...]
@@ -537,9 +538,10 @@
[...]/bb-slp-pr78205.c:13:8: note: transform load. ncopies = 1
[...]/bb-slp-pr78205.c:13:8: note: create vector_type-pointer variable to type: vector(4) double vectorizing a pointer ref: c[2]
[...]/bb-slp-pr78205.c:13:8: note: created &c[2]
-[...]/bb-slp-pr78205.c:13:8: note: add new stmt: vect__3.14_23 = MEM <vector(4) double> [(double *)&c];
-[...]/bb-slp-pr78205.c:13:8: note: add new stmt: vect__3.15_24 = MEM <vector(4) double> [(double *)&c + 32B];
-[...]/bb-slp-pr78205.c:13:8: note: add new stmt: vect__1.16_25 = VEC_PERM_EXPR <vect__3.14_23, vect__3.14_23, { 2, 3, 2, 3 }>;
+[...]/bb-slp-pr78205.c:13:8: note: add new stmt: vect__3.14_24 = MEM <vector(4) double> [(double *)&c];
+[...]/bb-slp-pr78205.c:13:8: note: add new stmt: _25 = MEM[(double *)&c + 32B];
+[...]/bb-slp-pr78205.c:13:8: note: add new stmt: vect__3.15_26 = {_25, 0.0, 0.0, 0.0};
+[...]/bb-slp-pr78205.c:13:8: note: add new stmt: vect__1.16_27 = VEC_PERM_EXPR <vect__3.14_24, vect__3.14_24, { 2, 3, 2, 3 }>;
[...]/bb-slp-pr78205.c:13:8: note: ------>vectorizing SLP node starting from: b[0] = _3;
[...]/bb-slp-pr78205.c:13:8: note: vect_is_simple_use: operand c[2], type of def: internal
[...]/bb-slp-pr78205.c:13:8: note: vect_is_simple_use: operand c[3], type of def: internal
[...]
@@ -580,18 +582,22 @@
double _4;
double _5;
vector(2) double _17;
+ double _20;
+ double _25;
<bb 2> [local count: 1073741824]:
vect__1.7_19 = MEM <vector(4) double> [(double *)&c];
- vect__1.9_21 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 0, 1, 0, 1 }>;
+ _20 = MEM[(double *)&c + 32B];
+ vect__1.9_22 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 0, 1, 0, 1 }>;
_1 = c[0];
_2 = c[1];
- MEM <vector(4) double> [(double *)&a] = vect__1.9_21;
- vect__3.14_23 = MEM <vector(4) double> [(double *)&c];
- vect__1.16_25 = VEC_PERM_EXPR <vect__3.14_23, vect__3.14_23, { 2, 3, 2, 3 }>;
+ MEM <vector(4) double> [(double *)&a] = vect__1.9_22;
+ vect__3.14_24 = MEM <vector(4) double> [(double *)&c];
+ _25 = MEM[(double *)&c + 32B];
+ vect__1.16_27 = VEC_PERM_EXPR <vect__3.14_24, vect__3.14_24, { 2, 3, 2, 3 }>;
_3 = c[2];
_4 = c[3];
- MEM <vector(4) double> [(double *)&b] = vect__1.16_25;
+ MEM <vector(4) double> [(double *)&b] = vect__1.16_27;
_5 = c[4];
_17 = {_5, _5};
MEM <vector(2) double> [(double *)&x] = _17;
--- bb-slp-pr78205.c.265t.optimized 2023-12-20 09:49:45.838344586 +0100
+++ bb-slp-pr78205.c.265t.optimized 2023-12-20 09:10:14.706300941 +0100
@@ -6,17 +6,17 @@
vector(4) double vect__1.16;
vector(4) double vect__1.9;
vector(4) double vect__1.7;
- double _5;
vector(2) double _17;
+ double _20;
<bb 2> [local count: 1073741824]:
vect__1.7_19 = MEM <vector(4) double> [(double *)&c];
- vect__1.9_21 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 0, 1, 0, 1 }>;
- MEM <vector(4) double> [(double *)&a] = vect__1.9_21;
- vect__1.16_25 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 2, 3, 2, 3 }>;
- MEM <vector(4) double> [(double *)&b] = vect__1.16_25;
- _5 = c[4];
- _17 = {_5, _5};
+ _20 = MEM[(double *)&c + 32B];
+ vect__1.9_22 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 0, 1, 0, 1 }>;
+ MEM <vector(4) double> [(double *)&a] = vect__1.9_22;
+ vect__1.16_27 = VEC_PERM_EXPR <vect__1.7_19, vect__1.7_19, { 2, 3, 2, 3 }>;
+ MEM <vector(4) double> [(double *)&b] = vect__1.16_27;
+ _17 = {_20, _20};
MEM <vector(2) double> [(double *)&x] = _17;
return;
--- bb-slp-pr78205.s 2023-12-20 09:49:45.846344519 +0100
+++ bb-slp-pr78205.s 2023-12-20 09:10:14.722300807 +0100
@@ -41,7 +41,17 @@
v_addc_co_u32 v7, s[22:23], 0, v7, s[22:23]
flat_load_dwordx2 v[6:7], v[6:7] offset:0
s_waitcnt 0
+ v_writelane_b32 v8, s12, 0
+ v_writelane_b32 v9, s13, 0
+ s_mov_b64 exec, 1
+ v_add_co_u32 v8, vcc, 32, v8
+ v_addc_co_u32 v9, vcc, 0, v9, vcc
+ flat_load_dwordx2 v[8:9], v[8:9]
+ s_waitcnt 0
+ v_readlane_b32 s12, v8, 0
+ v_readlane_b32 s13, v9, 0
s_mov_b64 s[18:19], 10
+ s_mov_b64 exec, 15
v_cndmask_b32 v0, 0, 4, s[18:19]
s_mov_b64 exec, 15
v_mov_b32 v11, v7
@@ -73,15 +83,6 @@
v_mov_b32 v5, s19
v_addc_co_u32 v5, s[22:23], 0, v5, s[22:23]
flat_store_dwordx2 v[4:5], v[6:7] offset:0
- v_writelane_b32 v4, s12, 0
- v_writelane_b32 v5, s13, 0
- s_mov_b64 exec, 1
- v_add_co_u32 v4, vcc, 32, v4
- v_addc_co_u32 v5, vcc, 0, v5, vcc
- flat_load_dwordx2 v[4:5], v[4:5]
- s_waitcnt 0
- v_readlane_b32 s12, v4, 0
- v_readlane_b32 s13, v5, 0
s_mov_b64 exec, 3
v_mov_b32 v6, s12
v_mov_b32 v7, s13
I haven't looked at the full context, but this appears to effectively
just move this block of code, and use different registers.
In:
+ s_mov_b64 exec, 15
v_cndmask_b32 v0, 0, 4, s[18:19]
s_mov_b64 exec, 15
... isn't the second (pre-existing) 's_mov_b64 exec, 15' now redundant,
though?
Grüße
Thomas
> PR tree-optimization/113073
> * tree-vect-stmts.cc (vectorizable_load): Properly ensure
> to exempt only vector-size aligned overreads.
> ---
> gcc/tree-vect-stmts.cc | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index fc6923cf68a..e9ff728dfd4 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -11476,7 +11476,9 @@ vectorizable_load (vec_info *vinfo,
> - (group_size * vf - gap), nunits))
> /* DR will be unused. */
> ltype = NULL_TREE;
> - else if (alignment_support_scheme == dr_aligned)
> + else if (known_ge (vect_align,
> + tree_to_poly_uint64
> + (TYPE_SIZE_UNIT (vectype))))
> /* Aligned access to excess elements is OK if
> at least one element is accessed in the
> scalar loop. */
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
next parent reply other threads:[~2023-12-20 9:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20231219123224.3D481385E459@sourceware.org>
2023-12-20 9:50 ` Thomas Schwinge [this message]
2023-12-20 10:21 ` Richard Biener
2023-12-20 13:44 ` Richard Biener
2023-12-20 14:31 ` Thomas Schwinge
2023-12-19 12:30 Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871qbh4300.fsf@euler.schwinge.homeip.net \
--to=thomas@codesourcery.com \
--cc=ams@codesourcery.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=julian@codesourcery.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).