Richard Biener <richard.guent...@gmail.com> writes:
> On Mon, Jul 3, 2017 at 9:49 AM, Richard Sandiford
> <richard.sandif...@linaro.org> wrote:
>> @@ -2070,8 +2143,7 @@ vect_find_same_alignment_drs (struct dat
>> if (dra == drb)
>> return;
>>
>> - if (!operand_equal_p (DR_BASE_OBJECT (dra), DR_BASE_OBJECT (drb),
>> - OEP_ADDRESS_OF)
>> + if (!operand_equal_p (DR_BASE_ADDRESS (dra), DR_BASE_ADDRESS (drb), 0)
>
> Why this change? It's semantically weaker after your change.
It's because the DR_BASE_OBJECT comes from the access_fn analysis
while the DR_BASE_ADDRESS comes from the innermost_loop_behavior.
I hadn't realised when adding the original code how different the
two were, and since all the other parts are based on the
innermost_loop_behavior, I think this check should be too.
E.g. it doesn't really make sense to compare DR_INITs based
on DR_BASE_OBJECT.
I guess it should have been a separate patch though.
>> || !operand_equal_p (DR_OFFSET (dra), DR_OFFSET (drb), 0)
>> || !operand_equal_p (DR_STEP (dra), DR_STEP (drb), 0))
>> return;
>> @@ -2129,6 +2201,7 @@ vect_analyze_data_refs_alignment (loop_v
>> vec<data_reference_p> datarefs = vinfo->datarefs;
>> struct data_reference *dr;
>>
>> + vect_record_base_alignments (vinfo);
>> FOR_EACH_VEC_ELT (datarefs, i, dr)
>> {
>> stmt_vec_info stmt_info = vinfo_for_stmt (DR_STMT (dr));
>> @@ -3327,7 +3400,8 @@ vect_analyze_data_refs (vec_info *vinfo,
>> {
>> struct data_reference *newdr
>> = create_data_ref (NULL, loop_containing_stmt (stmt),
>> - DR_REF (dr), stmt, maybe_scatter ? false : true);
>> + DR_REF (dr), stmt, !maybe_scatter,
>> + DR_IS_CONDITIONAL_IN_STMT (dr));
>> gcc_assert (newdr != NULL && DR_REF (newdr));
>> if (DR_BASE_ADDRESS (newdr)
>> && DR_OFFSET (newdr)
>> Index: gcc/tree-vect-slp.c
>> ===================================================================
>> --- gcc/tree-vect-slp.c 2017-07-03 08:20:56.404763323 +0100
>> +++ gcc/tree-vect-slp.c 2017-07-03 08:42:51.149380545 +0100
>> @@ -2358,6 +2358,7 @@ new_bb_vec_info (gimple_stmt_iterator re
>> gimple_stmt_iterator gsi;
>>
>> res = (bb_vec_info) xcalloc (1, sizeof (struct _bb_vec_info));
>> + new (&res->base_alignments) vec_base_alignments ();
>
> Ick. I'd rather make this proper C++ and do
>
> res = new _bb_vec_info;
>
> and add a constructor to the vec_info base initializing the hashtable.
> The above smells fishy.
I knew I pushing my luck with that one. I just didn't want to have to
convert the current xcalloc of loop_vec_info into a long list of explicit
zero initializers. (OK, we have a lot of explicit zero assignments already,
but certainly some fields rely on the calloc zeroing.)
> Looks like vec<> are happy with .create () being called on a zeroed struct.
>
> Alternatively add .create () to hashtable/map.
The difference is that vec<> is explicitly meant to be POD, whereas
hash_map isn't (and I don't think we want it to be).
Ah well. I'll do a separate pre-patch to C++-ify the structures.
Thanks,
Richard
