* Re: [patch] Vectorizer: bug fix for component ref support
[not found] <20041012175114.GA6378@redhat.com>
@ 2004-10-12 18:04 ` Dorit Naishlos
0 siblings, 0 replies; 9+ messages in thread
From: Dorit Naishlos @ 2004-10-12 18:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, Ira Rosen
> No, what I means is, if you attempt to support arbitrary ARRAY_TYPE
> references, do you find anything for which you fail?
maybe. we can give it a try on SPEC.
dorit
|---------+---------------------------->
| | Richard Henderson|
| | <rth@redhat.com> |
| | |
| | 12/10/2004 19:51 |
|---------+---------------------------->
>------------------------------------------------------------------------------------------------------------------------------|
| |
| To: Dorit Naishlos/Haifa/IBM@IBMIL |
| cc: gcc-patches@gcc.gnu.org, gcc-patches-owner@gcc.gnu.org, Ira Rosen/Haifa/IBM@IBMIL |
| Subject: Re: [patch] Vectorizer: bug fix for component ref support |
>------------------------------------------------------------------------------------------------------------------------------|
On Tue, Oct 12, 2004 at 07:42:03PM +0200, Dorit Naishlos wrote:
> nothing, otherwise we would have ICEd already - there's a gcc_assert
> following this check:
> is_array_ref = TREE_CODE (data_ref_base_type) == ARRAY_TYPE &&
> extra_checks);
> gcc_assert (is_ptr_ref || is_array_ref ...);
> It's really just a reminder of what we can/can't support.
No, what I means is, if you attempt to support arbitrary ARRAY_TYPE
references, do you find anything for which you fail?
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Vectorizer: bug fix for component ref support
2004-10-13 22:25 ` Dorit Naishlos
@ 2004-10-13 23:27 ` Richard Henderson
0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2004-10-13 23:27 UTC (permalink / raw)
To: Dorit Naishlos; +Cc: gcc-patches, Ira Rosen
On Wed, Oct 13, 2004 at 11:42:57PM +0200, Dorit Naishlos wrote:
> True. Shall we prepare a separate patch for that? (for 4.0?)
Yes please.
> ok to commit the above fix for now?
Yes.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Vectorizer: bug fix for component ref support
2004-10-13 15:54 ` Richard Henderson
@ 2004-10-13 22:25 ` Dorit Naishlos
2004-10-13 23:27 ` Richard Henderson
0 siblings, 1 reply; 9+ messages in thread
From: Dorit Naishlos @ 2004-10-13 22:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, Ira Rosen
> Any reason not to call vect_get_base_and_bit_offset always?
probably not. I just tested SPEC with this patch instead and it passed ok:
*************** vect_analyze_data_refs (loop_vec_info lo
*** 4933,4939 ****
{
case ARRAY_REF:
dr = analyze_array (stmt, TREE_OPERAND (symbl, 0),
DR_IS_READ(dr));
! STMT_VINFO_MEMTAG (stmt_info) = DR_BASE_NAME (dr);
break;
case VAR_DECL:
--- 4933,4941 ----
{
case ARRAY_REF:
dr = analyze_array (stmt, TREE_OPERAND (symbl, 0),
DR_IS_READ(dr));
! STMT_VINFO_MEMTAG (stmt_info) =
! vect_get_base_and_bit_offset (dr, DR_BASE_NAME
(dr), NULL_TREE,
! loop_vinfo, &offset,
&base_aligned_p);
break;
case VAR_DECL:
> I'll also note that vect_get_base_and_bit_offset should probably
> be rewritten using get_inner_reference.
True. Shall we prepare a separate patch for that? (for 4.0?)
ok to commit the above fix for now?
thanks,
dorit
|---------+---------------------------->
| | Richard Henderson|
| | <rth@redhat.com> |
| | |
| | 13/10/2004 17:49 |
|---------+---------------------------->
>----------------------------------------------------------------------------------------------------------------------------|
| |
| To: Ira Rosen/Haifa/IBM@IBMIL |
| cc: Dorit Naishlos/Haifa/IBM@IBMIL, gcc-patches@gcc.gnu.org |
| Subject: Re: [patch] Vectorizer: bug fix for component ref support |
>----------------------------------------------------------------------------------------------------------------------------|
On Wed, Oct 13, 2004 at 02:46:14PM +0200, Ira Rosen wrote:
> ! dr = analyze_array (stmt, TREE_OPERAND (symbl, 0),
DR_IS_READ(dr));
> ! if (TREE_CODE (DR_BASE_NAME (dr)) == COMPONENT_REF)
> ! STMT_VINFO_MEMTAG (stmt_info) =
> ! vect_get_base_and_bit_offset (dr,
DR_BASE_NAME (dr), NULL_TREE,
> !
loop_vinfo, &offset, &base_aligned_p);
Any reason not to call vect_get_base_and_bit_offset always?
Otherwise, I can see this == COMPONENT_REF test growing over time.
I'll also note that vect_get_base_and_bit_offset should probably
be rewritten using get_inner_reference. There are more cases to
handle than what you're currently doing. Mostly they show up with
Ada, so you won't have seen them yet.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Vectorizer: bug fix for component ref support
2004-10-13 13:04 ` Ira Rosen
@ 2004-10-13 15:54 ` Richard Henderson
2004-10-13 22:25 ` Dorit Naishlos
0 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2004-10-13 15:54 UTC (permalink / raw)
To: Ira Rosen; +Cc: Dorit Naishlos, gcc-patches
On Wed, Oct 13, 2004 at 02:46:14PM +0200, Ira Rosen wrote:
> ! dr = analyze_array (stmt, TREE_OPERAND (symbl, 0), DR_IS_READ(dr));
> ! if (TREE_CODE (DR_BASE_NAME (dr)) == COMPONENT_REF)
> ! STMT_VINFO_MEMTAG (stmt_info) =
> ! vect_get_base_and_bit_offset (dr, DR_BASE_NAME (dr), NULL_TREE,
> ! loop_vinfo, &offset, &base_aligned_p);
Any reason not to call vect_get_base_and_bit_offset always?
Otherwise, I can see this == COMPONENT_REF test growing over time.
I'll also note that vect_get_base_and_bit_offset should probably
be rewritten using get_inner_reference. There are more cases to
handle than what you're currently doing. Mostly they show up with
Ada, so you won't have seen them yet.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Vectorizer: bug fix for component ref support
[not found] <OF9A5E5AC4.AB654C57-ONC2256F2B.00626A5A-C2256F2B.0062BFF8@LocalDomain>
@ 2004-10-13 13:04 ` Ira Rosen
2004-10-13 15:54 ` Richard Henderson
0 siblings, 1 reply; 9+ messages in thread
From: Ira Rosen @ 2004-10-13 13:04 UTC (permalink / raw)
To: Dorit Naishlos, gcc-patches; +Cc: Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 676 bytes --]
> > No, what I means is, if you attempt to support arbitrary ARRAY_TYPE
> > references, do you find anything for which you fail?
>
> maybe. we can give it a try on SPEC.
I gave it a try on SPEC, and there were no ARRAY_TYPE refs other than what
we already currently expect, so I removed the extra check. O.K. with this
change?
Thanks,
Ira
Changelog entry:
2004-10-12 Ira Rosen <irar@il.ibm.com>
* tree-vectorizer.c (vect_analyze_data_refs): Call
vect_get_base_and_bit_offset to get memory tag for
component ref.
(vect_create_addr_base_for_vector_ref): Remove check of array
base code.
Patch:
(See attached file: diff_tag.oct13)
[-- Attachment #2: diff_tag.oct13 --]
[-- Type: application/octet-stream, Size: 3071 bytes --]
Index: ChangeLog
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ChangeLog,v
retrieving revision 2.5842
diff -c -3 -p -r2.5842 ChangeLog
*** ChangeLog 12 Oct 2004 08:30:40 -0000 2.5842
--- ChangeLog 13 Oct 2004 09:53:08 -0000
***************
*** 1,3 ****
--- 1,11 ----
+ 2004-10-12 Ira Rosen <irar@il.ibm.com>
+
+ * tree-vectorizer.c (vect_analyze_data_refs): Call
+ vect_get_base_and_bit_offset to get memory tag for
+ component ref.
+ (vect_create_addr_base_for_vector_ref): Remove check of array
+ base code.
+
2004-10-12 Joseph S. Myers <jsm@polyomino.org.uk>
PR c/17301
Index: tree-vectorizer.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-vectorizer.c,v
retrieving revision 2.14
diff -c -3 -p -r2.14 tree-vectorizer.c
*** tree-vectorizer.c 1 Oct 2004 09:59:00 -0000 2.14
--- tree-vectorizer.c 13 Oct 2004 09:53:09 -0000
*************** vect_create_addr_base_for_vector_ref (tr
*** 750,759 ****
is_ptr_ref = TREE_CODE (data_ref_base_type) == POINTER_TYPE
&& TREE_CODE (data_ref_base) == SSA_NAME;
! is_array_ref = TREE_CODE (data_ref_base_type) == ARRAY_TYPE
! && (TREE_CODE (data_ref_base) == VAR_DECL
! || TREE_CODE (data_ref_base) == COMPONENT_REF
! || TREE_CODE (data_ref_base) == ARRAY_REF);
is_addr_expr = TREE_CODE (data_ref_base) == ADDR_EXPR
|| TREE_CODE (data_ref_base) == PLUS_EXPR
|| TREE_CODE (data_ref_base) == MINUS_EXPR;
--- 750,756 ----
is_ptr_ref = TREE_CODE (data_ref_base_type) == POINTER_TYPE
&& TREE_CODE (data_ref_base) == SSA_NAME;
! is_array_ref = TREE_CODE (data_ref_base_type) == ARRAY_TYPE;
is_addr_expr = TREE_CODE (data_ref_base) == ADDR_EXPR
|| TREE_CODE (data_ref_base) == PLUS_EXPR
|| TREE_CODE (data_ref_base) == MINUS_EXPR;
*************** vect_analyze_data_refs (loop_vec_info lo
*** 3446,3451 ****
--- 3443,3450 ----
struct data_reference *dr;
tree tag;
tree address_base;
+ bool base_aligned_p;
+ tree offset;
if (vect_debug_details (NULL))
fprintf (dump_file, "\n<<vect_analyze_data_refs>>\n");
*************** vect_analyze_data_refs (loop_vec_info lo
*** 3554,3561 ****
switch (TREE_CODE (address_base))
{
case ARRAY_REF:
! dr = analyze_array (stmt, TREE_OPERAND (symbl, 0), DR_IS_READ(dr));
! STMT_VINFO_MEMTAG (stmt_info) = DR_BASE_NAME (dr);
break;
case VAR_DECL:
--- 3553,3565 ----
switch (TREE_CODE (address_base))
{
case ARRAY_REF:
! dr = analyze_array (stmt, TREE_OPERAND (symbl, 0), DR_IS_READ(dr));
! if (TREE_CODE (DR_BASE_NAME (dr)) == COMPONENT_REF)
! STMT_VINFO_MEMTAG (stmt_info) =
! vect_get_base_and_bit_offset (dr, DR_BASE_NAME (dr), NULL_TREE,
! loop_vinfo, &offset, &base_aligned_p);
! else
! STMT_VINFO_MEMTAG (stmt_info) = DR_BASE_NAME (dr);
break;
case VAR_DECL:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Vectorizer: bug fix for component ref support
2004-10-12 17:18 ` Dorit Naishlos
@ 2004-10-12 17:22 ` Richard Henderson
0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2004-10-12 17:22 UTC (permalink / raw)
To: Dorit Naishlos; +Cc: gcc-patches, Ira Rosen
On Tue, Oct 12, 2004 at 07:06:09PM +0200, Dorit Naishlos wrote:
> It may be that in its current form it actually allows all possible forms
> of arrays we can get in GIMPLE (is that the case?)
I don't know, but that's certainly the most desirable solution.
What happens if you remove the extra checks?
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Vectorizer: bug fix for component ref support
2004-10-12 16:58 ` Richard Henderson
@ 2004-10-12 17:18 ` Dorit Naishlos
2004-10-12 17:22 ` Richard Henderson
0 siblings, 1 reply; 9+ messages in thread
From: Dorit Naishlos @ 2004-10-12 17:18 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, Ira Rosen
> Why are we checking anything other than ARRAY_TYPE?
just a sanity check to make sure that we are getting here only array forms
that we know that we support. It used to be only arrays which base is a
VAR_DECL; then we added struct support, multidimensional support, and
pointers support, so this check was gradually relaxed. It may be that in
its current form it actually allows all possible forms of arrays we can get
in GIMPLE (is that the case?) and is therefore redundant or should go under
ENABLE_CHECKING. OK if we do that?
dorit
Richard Henderson
<rth@redhat.com> To: Ira Rosen/Haifa/IBM@IBMIL
cc: gcc-patches@gcc.gnu.org, Dorit Naishlos/Haifa/IBM@IBMIL
12/10/2004 18:49 Subject: Re: [patch] Vectorizer: bug fix for component ref support
On Tue, Oct 12, 2004 at 03:21:30PM +0200, Ira Rosen wrote:
> is_array_ref = TREE_CODE (data_ref_base_type) == ARRAY_TYPE
> && (TREE_CODE (data_ref_base) == VAR_DECL
> || TREE_CODE (data_ref_base) == COMPONENT_REF
> ! || TREE_CODE (data_ref_base) == ARRAY_REF
> ! || TREE_CODE (data_ref_base) == INDIRECT_REF);
Why are we checking anything other than ARRAY_TYPE?
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Vectorizer: bug fix for component ref support
2004-10-12 13:24 Ira Rosen
@ 2004-10-12 16:58 ` Richard Henderson
2004-10-12 17:18 ` Dorit Naishlos
0 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2004-10-12 16:58 UTC (permalink / raw)
To: Ira Rosen; +Cc: gcc-patches, Dorit Naishlos
On Tue, Oct 12, 2004 at 03:21:30PM +0200, Ira Rosen wrote:
> is_array_ref = TREE_CODE (data_ref_base_type) == ARRAY_TYPE
> && (TREE_CODE (data_ref_base) == VAR_DECL
> || TREE_CODE (data_ref_base) == COMPONENT_REF
> ! || TREE_CODE (data_ref_base) == ARRAY_REF
> ! || TREE_CODE (data_ref_base) == INDIRECT_REF);
Why are we checking anything other than ARRAY_TYPE?
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch] Vectorizer: bug fix for component ref support
@ 2004-10-12 13:24 Ira Rosen
2004-10-12 16:58 ` Richard Henderson
0 siblings, 1 reply; 9+ messages in thread
From: Ira Rosen @ 2004-10-12 13:24 UTC (permalink / raw)
To: gcc-patches; +Cc: Dorit Naishlos
[-- Attachment #1: Type: text/plain, Size: 741 bytes --]
Hi,
This patch fixes a bug in vectorization support for component refs. In case
of data reference &a.b[i], the memory tag should be a (and not a.b, as it
was previously). It also relaxes an assert that was too strict. The first
problem caused ICEs in SPECs gcc and fma benchmarks, and the second one
caused ICE in mesa benchmark.
Bootstrapped and tested on ppc-darwin. O.K. for mainline?
Thanks,
Ira
Changelog entry:
2004-10-12 Ira Rosen <irar@il.ibm.com>
* tree-vectorizer.c (vect_analyze_data_refs): Call
vect_get_base_and_bit_offset to get memory tag for
component ref.
(vect_create_addr_base_for_vector_ref): Allow indirect
ref to be an array base.
Patch:
(See attached file: diff_tag.oct12)
[-- Attachment #2: diff_tag.oct12 --]
[-- Type: application/octet-stream, Size: 2412 bytes --]
Index: tree-vectorizer.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-vectorizer.c,v
retrieving revision 2.14
diff -c -3 -p -r2.14 tree-vectorizer.c
*** tree-vectorizer.c 1 Oct 2004 09:59:00 -0000 2.14
--- tree-vectorizer.c 12 Oct 2004 11:29:40 -0000
*************** vect_create_addr_base_for_vector_ref (tr
*** 753,759 ****
is_array_ref = TREE_CODE (data_ref_base_type) == ARRAY_TYPE
&& (TREE_CODE (data_ref_base) == VAR_DECL
|| TREE_CODE (data_ref_base) == COMPONENT_REF
! || TREE_CODE (data_ref_base) == ARRAY_REF);
is_addr_expr = TREE_CODE (data_ref_base) == ADDR_EXPR
|| TREE_CODE (data_ref_base) == PLUS_EXPR
|| TREE_CODE (data_ref_base) == MINUS_EXPR;
--- 753,760 ----
is_array_ref = TREE_CODE (data_ref_base_type) == ARRAY_TYPE
&& (TREE_CODE (data_ref_base) == VAR_DECL
|| TREE_CODE (data_ref_base) == COMPONENT_REF
! || TREE_CODE (data_ref_base) == ARRAY_REF
! || TREE_CODE (data_ref_base) == INDIRECT_REF);
is_addr_expr = TREE_CODE (data_ref_base) == ADDR_EXPR
|| TREE_CODE (data_ref_base) == PLUS_EXPR
|| TREE_CODE (data_ref_base) == MINUS_EXPR;
*************** vect_analyze_data_refs (loop_vec_info lo
*** 3446,3451 ****
--- 3447,3454 ----
struct data_reference *dr;
tree tag;
tree address_base;
+ bool base_aligned_p;
+ tree offset;
if (vect_debug_details (NULL))
fprintf (dump_file, "\n<<vect_analyze_data_refs>>\n");
*************** vect_analyze_data_refs (loop_vec_info lo
*** 3554,3561 ****
switch (TREE_CODE (address_base))
{
case ARRAY_REF:
! dr = analyze_array (stmt, TREE_OPERAND (symbl, 0), DR_IS_READ(dr));
! STMT_VINFO_MEMTAG (stmt_info) = DR_BASE_NAME (dr);
break;
case VAR_DECL:
--- 3557,3569 ----
switch (TREE_CODE (address_base))
{
case ARRAY_REF:
! dr = analyze_array (stmt, TREE_OPERAND (symbl, 0), DR_IS_READ(dr));
! if (TREE_CODE (DR_BASE_NAME (dr)) == COMPONENT_REF)
! STMT_VINFO_MEMTAG (stmt_info) =
! vect_get_base_and_bit_offset (dr, DR_BASE_NAME (dr), NULL_TREE,
! loop_vinfo, &offset, &base_aligned_p);
! else
! STMT_VINFO_MEMTAG (stmt_info) = DR_BASE_NAME (dr);
break;
case VAR_DECL:
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2004-10-13 23:03 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20041012175114.GA6378@redhat.com>
2004-10-12 18:04 ` [patch] Vectorizer: bug fix for component ref support Dorit Naishlos
[not found] <OF9A5E5AC4.AB654C57-ONC2256F2B.00626A5A-C2256F2B.0062BFF8@LocalDomain>
2004-10-13 13:04 ` Ira Rosen
2004-10-13 15:54 ` Richard Henderson
2004-10-13 22:25 ` Dorit Naishlos
2004-10-13 23:27 ` Richard Henderson
2004-10-12 13:24 Ira Rosen
2004-10-12 16:58 ` Richard Henderson
2004-10-12 17:18 ` Dorit Naishlos
2004-10-12 17:22 ` Richard Henderson
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).