On Fri, Sep 12, 2014 at 1:09 PM, Bernd Schmidt <ber...@codesourcery.com> wrote:
> On 09/11/2014 01:39 PM, Richard Biener wrote:
>
>> Seeing this it would be nice to abstract away the exact place we store
>> the address-space in a memory reference. So - can you add a helper
>> reference_addr_space () please? Thus do
>>
>> addr_space_t as = reference_addr_space (scalar_dest);
>
>
> Ok, no problem.
Thanks.
>> but then I wonder why not simply build the correct vector types
>> in the first place in vect_analyze_data_refs?
>
>
> We do, kind of. But there are two problems, the first one addressed here in
> the patch:
> - data_ref = build2 (MEM_REF, TREE_TYPE (vec_oprnd),
> dataref_ptr,
> + data_ref = build2 (MEM_REF, vectype, dataref_ptr,
>
> We use the wrong vector type (vectype has the right address space, but
> vec_oprnd is an SSA_NAME).
Ah, ok. That part is fine then.
> The second problem is our reprentation of vector types. When given
> <address-space-5> unsigned int
> make_vector_type makes
> <address-space-5> vector(2) unsigned int
> rather than
> <address-space-5> vector(2) <address-space-5> unsigned int
>
> Since we use elem_type in a few ways in tree_vectorizable_stmt, that looks
> like it would cause problems, but it probably can be addressed in a simpler
> way than what I originally had:
>
> Index: gomp-4_0-branch/gcc/tree-vect-stmts.c
> ===================================================================
> --- gomp-4_0-branch.orig/gcc/tree-vect-stmts.c
> +++ gomp-4_0-branch/gcc/tree-vect-stmts.c
> @@ -5037,7 +5037,8 @@ vectorizable_store (gimple stmt, gimple_
> return false;
> }
>
> - elem_type = TREE_TYPE (vectype);
> + elem_type = apply_addr_space_to_type (TREE_TYPE (vectype),
> + TYPE_ADDR_SPACE (vectype));
> vec_mode = TYPE_MODE (vectype);
>
> /* FORNOW. In some cases can vectorize even if data-type not supported
> @@ -5692,7 +5693,8 @@ vectorizable_load (gimple stmt, gimple_s
> if (!STMT_VINFO_DATA_REF (stmt_info))
> return false;
>
> - elem_type = TREE_TYPE (vectype);
> + elem_type = apply_addr_space_to_type (TREE_TYPE (vectype),
> + TYPE_ADDR_SPACE (vectype));
> mode = TYPE_MODE (vectype);
>
> /* FORNOW. In some cases can vectorize even if data-type not supported
>
> The other alternative is to fix up make_vector_type. That could look like
> this:
>
> Index: gomp-4_0-branch/gcc/tree.c
> ===================================================================
> --- gomp-4_0-branch.orig/gcc/tree.c
> +++ gomp-4_0-branch/gcc/tree.c
> @@ -9470,9 +9470,13 @@ make_vector_type (tree innertype, int nu
> inner type. Use it to build the variant we return. */
> if ((TYPE_ATTRIBUTES (innertype) || TYPE_QUALS (innertype))
> && TREE_TYPE (t) != innertype)
> - return build_type_attribute_qual_variant (t,
> - TYPE_ATTRIBUTES (innertype),
> - TYPE_QUALS (innertype));
> + {
> + t = build_type_attribute_qual_variant (t,
> + TYPE_ATTRIBUTES (innertype),
> + TYPE_QUALS (innertype));
> + TREE_TYPE (t) = apply_addr_space_to_type (TREE_TYPE (t),
> + TYPE_ADDR_SPACE
> (innertype));
> + }
>
> return t;
> }
>
> Let me know what you prefer.
Hmm, neither I suppose. COMPLEX_TYPEs are also built
with main-variant component type and I suspect the same for
ARRAY_TYPEs. I see the address-space on types as
artifact that comes from Frontend support (aka parsing).
>> Or apply the addr-space to the memory reference with a new helper
>> reference_apply_addr_space
>>
>> - data_ref = build2 (MEM_REF, TREE_TYPE (vec_oprnd),
>> dataref_ptr,
>> dataref_offset
>> ? dataref_offset
>> : build_int_cst
>> (reference_alias_ptr_type
>> ..
>> reference_apply_addr_space (data_ref, as);
>>
>> at least that's how it's abstracted on the RTL side. I think I'd prefer
>> if things would be working that way on the tree level, too.
>
>
> I'm adapting my other patches to use such a function, but in this particular
> case I think it would be best to fix up the types in advance since we build
> accesses in several places and especially vectorizable_load also seems to
> create operations on the pointer type. Are you ok with that?
Fixing up the vector type in advance is ok with me but I'd like us to
move away from address-space-on-types.
Thanks,
Richard.
>
> Bernd
>
