[PATCH GCC][09/13]Simply cost model merges partitions with the same references
Richard Biener
richard.guenther@gmail.com
Fri Jun 23 10:48:00 GMT 2017
On Fri, Jun 23, 2017 at 12:19 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Mon, Jun 19, 2017 at 4:20 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Mon, Jun 19, 2017 at 3:40 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>> On Wed, Jun 14, 2017 at 2:54 PM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> On Mon, Jun 12, 2017 at 7:03 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>>>>> Hi,
>>>>> Current primitive cost model merges partitions with data references sharing the same
>>>>> base address. I believe it's designed to maximize data reuse in distribution, but
>>>>> that should be done by dedicated data reusing algorithm. At this stage of merging,
>>>>> we should be conservative and only merge partitions with the same references.
>>>>> Bootstrap and test on x86_64 and AArch64. Is it OK?
>>>>
>>>> Well, I'd say "conservative" is merging more, not less. For example
>>>> splitting a[i+1] from a[i]
>>>> would be bad(?), so I'd see to allow unequal DR_INIT as "equal" for
>>>> merging. Maybe
>>>> DR_INIT within a cacheline or so.
>>>>
>>>> How many extra distributions in say SPEC do you get from this change alone?
>>> Hi,
>>> I collected data for spec2006 only with/without this patch. I am a
>>> bit surprised that it doesn't change the number of distributed loops.
>>>>
>>>> It shows also that having partition->reads_and_writes would be nice
>>>> ... the code duplication
>>> Yeah, I merged read/write data references in previous patch, now this
>>> duplication is gone. Update patch attached. Is it OK?
>>
>> + gcc_assert (i < datarefs_vec.length ());
>> + dr1 = datarefs_vec[i];
>>
>> these asserts are superfluous -- vec::operator[] does them as well.
>>
>> Ok if you remove them.
> Done.
> I realized I made mistakes when measuring the impact of this patch.
> This patch only apparently causes failure of
> gcc.dg/tree-ssa/ldist-6.c, so here is the updated patch. I also
> collected the number of distributed loops in spec2k6 as below:
> trunk: 5882
> only this patch: 7130
> whole patch series: 5237
> So the conclusion is, this patch does aggressive distribution like
> ldist-6.c, which means worse data-locality. The following patch does
> more fusion which mitigates impact of this patch and results in
> conservative distribution overall.
What changed in the patch? Did you attach the correct one?
I'm not sure ldist-6.c is a "valid" testcase but I didn't try to see
where it was reduced from.
> But as we lack of data locality
> cost model, ldist-6.c remains failed even after applying whole patch
> series. Hmm, a cache-sensitive cost model is need for several passes
> now, distribution, prefetch and (possible) interchange.
> Richard, do you have second comment based on the new data?
I expected the "only this patch" result somewhat, as said, I'd have
allowed "related" references to fuse by not requiring equal
DR_INIT for example.
I suggest to go forward with it in its current form. We can tweak the
cost model later.
Thanks,
Richard.
> Thanks,
> bin
> 2017-06-20 Bin Cheng <bin.cheng@arm.com>
>
> * tree-loop-distribution.c (ref_base_address): Delete.
> (similar_memory_accesses): Rename ...
> (share_memory_accesses): ... to this. Check if partitions access
> the same memory reference.
> (distribute_loop): Call share_memory_accesses.
>
> gcc/testsuite/ChangeLog
> 2017-06-20 Bin Cheng <bin.cheng@arm.com>
>
> * gcc.dg/tree-ssa/ldist-6.c: XFAIL.
More information about the Gcc-patches
mailing list