Re: [PATCH GCC][10/13]Compute and cache data dependence relation

[Richard Biener <richard.guenther@gmail.com>]

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.

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.