On Thu, Nov 14, 2013 at 12:25 AM, Yufeng Zhang <yufeng.zh...@arm.com> wrote:
> Hi Bill,
>
>
> On 11/13/13 20:54, Bill Schmidt wrote:
>>
>> Hi Yufeng,
>>
>> The second version of your original patch is ok with me with the
>> following changes. Sorry for the little side adventure into the
>> next-interp logic; in the end that's going to hurt more than it helps in
>> this case. Thanks for having a look at it, anyway. Thanks also for
>> cleaning up this version to be less intrusive to common interfaces; I
>> appreciate it.
>
>
> Thanks a lot for the review. I've attached an updated patch with the
> suggested changes incorporated.
>
> For the next-interp adventure, I was quite happy to do the experiment; it's
> a good chance of gaining insight into the pass. Many thanks for your prompt
> replies and patience in guiding!
>
>
>> Everything else looks OK to me. Please ask Richard for final approval,
>> as I'm not a maintainer.
>
>
> Hi Richard, would you be happy to OK the patch?
Hmm,
+static tree
+get_alternative_base (tree base)
+{
+ tree *result = (tree *) pointer_map_contains (alt_base_map, base);
+
+ if (result == NULL)
+ {
+ tree expr;
+ aff_tree aff;
+
+ tree_to_aff_combination_expand (base, TREE_TYPE (base),
+ &aff, &name_expansions);
+ aff.offset = tree_to_double_int (integer_zero_node);
+ expr = aff_combination_to_tree (&aff);
+
+ result = (tree *) pointer_map_insert (alt_base_map, base);
+ gcc_assert (!*result);
I believe this cache will never hit (unless you repeatedly ask for
the exact same statement?) - any non-trivial 'base' trees are
not shared and thus not pointer equivalent.
Also using tree_to_aff_combination_expand to get at - what
exactly? The address with any constant offset stripped?
Where do you re-construct that offset? That is, aff.offset,
which you definitely need to get at a candidate?
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-slsr" } */
+
+typedef int arr_2[50][50];
+
+void foo (arr_2 a2, int v1)
+{
+ int i, j;
+
+ i = v1 + 5;
+ j = i;
+ a2 [i-10] [j] = 2;
+ a2 [i] [j++] = i;
+ a2 [i+20] [j++] = i;
+ a2 [i-3] [i-1] += 1;
+ return;
+}
+
+/* { dg-final { scan-tree-dump-times "MEM" 5 "slsr" } } */
+/* { dg-final { cleanup-tree-dump "slsr" } } */
scanning for 5 MEMs looks non-sensical. What transform do
you expect? I see other slsr testcases do similar non-sensical
checking which is bad, too.
Richard.
> Regards,
>
> Yufeng
>
> gcc/
>
> * gimple-ssa-strength-reduction.c: Include tree-affine.h.
> (name_expansions): New static variable.
> (alt_base_map): Ditto.
> (get_alternative_base): New function.
> (find_basis_for_candidate): For CAND_REF, optionally call
> find_basis_for_base_expr with the returned value from
> get_alternative_base.
> (record_potential_basis): Add new parameter 'base' of type 'tree';
> add an assertion of non-NULL base; use base to set node->base_expr.
>
> (alloc_cand_and_find_basis): Update; call record_potential_basis
> for CAND_REF with the returned value from get_alternative_base.
> (execute_strength_reduction): Call pointer_map_create for
> alt_base_map; call free_affine_expand_cache with &name_expansions.
>
> gcc/testsuite/
>
> * gcc.dg/tree-ssa/slsr-41.c: New test.
