This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Reducing number of alias checks in vectorization.
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Cong Hou <congh at google dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>
- Date: Wed, 2 Oct 2013 08:35:28 +0200
- Subject: Re: [PATCH] Reducing number of alias checks in vectorization.
- Authentication-results: sourceware.org; auth=none
- References: <CAK=A3=3sjM_MCqDoXwBXPsDiBDRGPuGh3oBkBOt_3685=dUXPw at mail dot gmail dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Tue, Oct 01, 2013 at 07:12:54PM -0700, Cong Hou wrote:
> --- gcc/tree-vect-loop-manip.c (revision 202662)
> +++ gcc/tree-vect-loop-manip.c (working copy)
Your mailer ate all the tabs, so the formatting of the whole patch
can't be checked.
> @@ -19,6 +19,10 @@ You should have received a copy of the G
> along with GCC; see the file COPYING3. If not see
> <http://www.gnu.org/licenses/>. */
>
> +#include <vector>
> +#include <utility>
> +#include <algorithm>
Why? GCC has it's vec.h vectors, why don't you use those?
There is even qsort method for you in there. And for pairs, you can
easily just use structs with two members as structure elements in the
vector.
> +struct dr_addr_with_seg_len
> +{
> + dr_addr_with_seg_len (data_reference* d, tree addr, tree off, tree len)
> + : dr (d), basic_addr (addr), offset (off), seg_len (len) {}
> +
> + data_reference* dr;
Space should be before *, not after it.
> + if (TREE_CODE (p11.offset) != INTEGER_CST
> + || TREE_CODE (p21.offset) != INTEGER_CST)
> + return p11.offset < p21.offset;
If offset isn't INTEGER_CST, you are comparing the pointer values?
That is never a good idea, then compilation will depend on how say address
space randomization randomizes virtual address space. GCC needs to have
reproduceable compilations.
> + if (int_cst_value (p11.offset) != int_cst_value (p21.offset))
> + return int_cst_value (p11.offset) < int_cst_value (p21.offset);
This is going to ICE whenever the offsets wouldn't fit into a
HOST_WIDE_INT.
I'd say you just shouldn't put into the vector entries where offset isn't
host_integerp, those would never be merged with other checks, or something
similar.
Jakub