On Mon, Sep 2, 2013 at 8:53 PM, Bernd Edlinger
<bernd.edlin...@hotmail.de> wrote:
> On Fri, 30 Aug 2013 12:34:51 Richard Biener wrote:
>> On Tue, Aug 20, 2013 at 1:01 PM, Bernd Edlinger
>> <bernd.edlin...@hotmail.de> wrote:
>>> This patch fixes a bug in the vectorized pointer arithmetic in the forwprop
>>> optimization pass.
>>>
>>> Although it seems to be impossible to create a vector of pointers with the
>>> __attribute__((vector_size(x))) syntax, the vector loop optimization
>>> together
>>> with the loop unrolling can create code which adds a vector of ptroff_t
>>> repeatedly to a vector of pointers.
>>>
>>> The code in tree-ssa-forwprop.c handles program transformations of the
>>> form (PTR +- CST1) +- CST2 => PTR +- (CST1+-CST2) where PTR is
>>> a vector of pointers and CST1 and CST2 are vectors of ptroff_t, without the
>>> fix the result type of CST1+-CST2 was vector of pointer, which causes the
>>> ICE in tree-cfg.c, which sees an undefined pointer + pointer operation.
>>>
>>> Additionally the check in tree-cfg.c allows expressions of the form CST -
>>> PTR,
>>> which is clearly wrong. That should be fixed too.
>>>
>>> The patch was bootstrapped and regression tested on i686-pc-linux-gnu.
>>
>> It seems to me that the forwprop code does not handle the fact that we
>> are permissive as to using PLUS_EXPR instead of POINTER_PLUS_EXPR
>> for vector pointer - offset additions. So instead of doing this dance the
>> better (and more easily backportable) fix is to simply disable the transform
>> on pointer valued PLUS_EXPR. Like with
>>
>> if (POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
>> return false;
>>
>> at the beginning of the function.
>>
>
> the condition would have to be:
>
> if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt))) == VECTOR_TYPE
> && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (gimple_assign_lhs (stmt)))))
> return false;
>
> I tried this, and it fixes the ICE. However the generated code was still
> vectorized but
> much less efficient and used more registers.
>
> Fortunately there will be no need to backport this, since this bug does not
> happen in
> gcc 4.8.2 I checked that. I believe it was introduced with the checkin
> r200059 by
> Marc Glisse where associate_plusminus was enhanced to handle vector values.
> Before that only TREE_CODE (rhs2) == INTEGER_CST was handled.
>
> Frankly I would prefer the initial version of the patch, because the code is
> more
> efficient this way. The vector data is folded correctly, only the data type
> was wrong
> and triggered the ICE in tree-cfg.c.
>
> Please advise.
I'd rather go with the simple fix as the issue in forwprop is at least
latent. We can
improve on the code-gen as followup where I believe handling of
POINTER_PLUS_EXPR
would need to be added (that we avoid POINTER_PLUS_EXPR for vectors is a bug).
That can be done in a way to cover the vector case properly. Or
finally properly
use POINTER_PLUS_EXPR for vectors or make the vectorizer not use pointer
types but a corresponding unsigned integer type for them (that would also fix
the original bug of course). Like with (untested)
Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c (revision 202196)
+++ gcc/tree-vect-stmts.c (working copy)
@@ -6179,8 +6179,7 @@ get_vectype_for_scalar_type_and_size (tr
corresponding to that mode. The theory is that any use that
would cause problems with this will disable vectorization anyway. */
else if (!SCALAR_FLOAT_TYPE_P (scalar_type)
- && !INTEGRAL_TYPE_P (scalar_type)
- && !POINTER_TYPE_P (scalar_type))
+ && !INTEGRAL_TYPE_P (scalar_type))
scalar_type = lang_hooks.types.type_for_mode (inner_mode, 1);
/* We can't build a vector type of elements with alignment bigger than
actually that would be my preference here ...
Thanks,
Richard.
> Thanks
> Bernd.
>
>> The real fix is of course to make vector pointer operations properly
>> use POINTER_PLUS_EXPR ...
>>
>> Richard.
>>
>>
>>
>>> Regards
>>> Bernd Edlinger
