On Tue, Jun 20, 2017 at 12:32 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Jun 20, 2017 at 11:15 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Fri, Jun 16, 2017 at 11:03 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On Mon, Jun 12, 2017 at 7:03 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>>>> Hi,
>>>> This patch computes and caches data dependence relation in a hash table
>>>> so that it can be queried multiple times later for partition dependence
>>>> check.
>>>> Bootstrap and test on x86_64 and AArch64.  Is it OK?
>>>
>>> +/* Vector of data dependence relations.  */
>>> +static vec<ddr_p> *ddrs_vec;
>>> +
>>> +/* Hash table for data dependence relation in the loop to be distributed.  */
>>> +static hash_table<ddr_entry_hasher> *ddrs_table;
>>>
>>> avoid the extra indirection.
>>>
>>> +/* Hashtable entry for data reference relation.  */
>>> +struct ddr_entry
>>> +{
>>> +  data_reference_p a;
>>> +  data_reference_p b;
>>> +  ddr_p ddr;
>>> +  hashval_t hash;
>>> +};
>>> ...
>>> +/* Hash table equality function for data reference relation.  */
>>> +
>>> +inline bool
>>> +ddr_entry_hasher::equal (const ddr_entry *entry1, const ddr_entry *entry2)
>>> +{
>>> +  return (entry1->hash == entry2->hash
>>> +         && DR_STMT (entry1->a) == DR_STMT (entry2->a)
>>> +         && DR_STMT (entry1->b) == DR_STMT (entry2->b)
>>> +         && operand_equal_p (DR_REF (entry1->a), DR_REF (entry2->a), 0)
>>> +         && operand_equal_p (DR_REF (entry1->b), DR_REF (entry2->b), 0));
>>> +}
>>>
>>> what's the issue with using hash_table <ddr_p> with a custom hasher?
>>> That is, simply key on the dataref pointers (hash them, compare those
>>> for equality)?
>>>
>>> Your scheme looks too complicated / expensive to me ...
>>>
>>> You can drop ddrs_vec needed only for memory removal if you traverse
>>> the hashtable.
>> Thanks for reviewing.  Patch simplified as suggested.  Is it OK?
>
> +inline hashval_t
> +ddr_hasher::hash (const data_dependence_relation *ddr)
> +{
> +  return iterative_hash_object (DDR_A (ddr),
> +                               iterative_hash_object (DDR_B (ddr), 0));
> +}
> +
>
> please use
>
>     inchash::hash h;
>     h.add_ptr (DDR_A (ddr));
>     h.add_ptr (DDR_B (ddr));
>     return h.end ();
>
> Ok with that change.
Done, patch updated.

Thanks,
bin
>
> Richard.
>
>> Thanks,
>> bin
>> 2017-06-17  Bin Cheng  <bin.cheng@arm.com>
>>
>>     * tree-loop-distribution.c (struct ddr_hasher): New.
>>     (ddr_hasher::hash, ::equal, get_data_dependence): New function.
>>     (ddrs_table): New.
>>     (classify_partition): Call get_data_dependence.
>>     (pg_add_dependence_edges): Ditto.
>>     (distribute_loop): Release data dependence hash table.