| Message ID | 87eflnc3dh.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Check array indices in object_address_invariant_in_loop_p (PR 84357) | expand |
On Wed, Feb 14, 2018 at 10:44 AM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > object_address_invariant_in_loop_p ignored ARRAY_REF indices on > the basis that: > > /* Index of the ARRAY_REF was zeroed in analyze_indices, thus we only > need to check the stride and the lower bound of the reference. */ > > That was true back in 2007 when the code was added: > > static void > dr_analyze_indices (struct data_reference *dr, struct loop *nest) > { > [...] > while (handled_component_p (aref)) > { > if (TREE_CODE (aref) == ARRAY_REF) > { > op = TREE_OPERAND (aref, 1); > access_fn = analyze_scalar_evolution (loop, op); > access_fn = resolve_mixers (nest, access_fn); > VEC_safe_push (tree, heap, access_fns, access_fn); > > TREE_OPERAND (aref, 1) = build_int_cst (TREE_TYPE (op), 0); > } > > aref = TREE_OPERAND (aref, 0); > } > > but the assignment was removed a few years ago. GCC 4.7! > We were therefore > treating "two->arr[i]" and "three->arr[i]" as loop invariant. > > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. > OK to install? Ok. Thanks, Richard. > Richard > > > 2018-02-14 Richard Sandiford <richard.sandiford@linaro.org> > > gcc/ > PR tree-optimization/84357 > * tree-data-ref.c (object_address_invariant_in_loop_p): Check > operand 1 of an ARRAY_REF too. > > gcc/testsuite/ > PR tree-optimization/84357 > * gcc.dg/vect/pr84357.c: New test. > > Index: gcc/tree-data-ref.c > =================================================================== > --- gcc/tree-data-ref.c 2018-02-08 15:16:21.784407397 +0000 > +++ gcc/tree-data-ref.c 2018-02-14 09:42:14.801095011 +0000 > @@ -2200,13 +2200,10 @@ object_address_invariant_in_loop_p (cons > { > if (TREE_CODE (obj) == ARRAY_REF) > { > - /* Index of the ARRAY_REF was zeroed in analyze_indices, thus we only > - need to check the stride and the lower bound of the reference. */ > - if (chrec_contains_symbols_defined_in_loop (TREE_OPERAND (obj, 2), > - loop->num) > - || chrec_contains_symbols_defined_in_loop (TREE_OPERAND (obj, 3), > - loop->num)) > - return false; > + for (int i = 1; i < 4; ++i) > + if (chrec_contains_symbols_defined_in_loop (TREE_OPERAND (obj, i), > + loop->num)) > + return false; > } > else if (TREE_CODE (obj) == COMPONENT_REF) > { > Index: gcc/testsuite/gcc.dg/vect/pr84357.c > =================================================================== > --- /dev/null 2018-02-10 09:05:46.714416790 +0000 > +++ gcc/testsuite/gcc.dg/vect/pr84357.c 2018-02-14 09:42:14.800095067 +0000 > @@ -0,0 +1,31 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-Wall" } */ > + > +#define COUNT 32 > + > +typedef struct s1 { > + unsigned char c; > +} s1; > + > +typedef struct s2 > +{ > + char pad; > + s1 arr[COUNT]; > +} s2; > + > +typedef struct s3 { > + s1 arr[COUNT]; > +} s3; > + > +s2 * get_s2(); > +s3 * gActiveS3; > +void foo() > +{ > + s3 * three = gActiveS3; > + s2 * two = get_s2(); > + > + for (int i = 0; i < COUNT; i++) > + { > + two->arr[i].c = three->arr[i].c; > + } > +}
Index: gcc/tree-data-ref.c =================================================================== --- gcc/tree-data-ref.c 2018-02-08 15:16:21.784407397 +0000 +++ gcc/tree-data-ref.c 2018-02-14 09:42:14.801095011 +0000 @@ -2200,13 +2200,10 @@ object_address_invariant_in_loop_p (cons { if (TREE_CODE (obj) == ARRAY_REF) { - /* Index of the ARRAY_REF was zeroed in analyze_indices, thus we only - need to check the stride and the lower bound of the reference. */ - if (chrec_contains_symbols_defined_in_loop (TREE_OPERAND (obj, 2), - loop->num) - || chrec_contains_symbols_defined_in_loop (TREE_OPERAND (obj, 3), - loop->num)) - return false; + for (int i = 1; i < 4; ++i) + if (chrec_contains_symbols_defined_in_loop (TREE_OPERAND (obj, i), + loop->num)) + return false; } else if (TREE_CODE (obj) == COMPONENT_REF) { Index: gcc/testsuite/gcc.dg/vect/pr84357.c =================================================================== --- /dev/null 2018-02-10 09:05:46.714416790 +0000 +++ gcc/testsuite/gcc.dg/vect/pr84357.c 2018-02-14 09:42:14.800095067 +0000 @@ -0,0 +1,31 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-Wall" } */ + +#define COUNT 32 + +typedef struct s1 { + unsigned char c; +} s1; + +typedef struct s2 +{ + char pad; + s1 arr[COUNT]; +} s2; + +typedef struct s3 { + s1 arr[COUNT]; +} s3; + +s2 * get_s2(); +s3 * gActiveS3; +void foo() +{ + s3 * three = gActiveS3; + s2 * two = get_s2(); + + for (int i = 0; i < COUNT; i++) + { + two->arr[i].c = three->arr[i].c; + } +}
object_address_invariant_in_loop_p ignored ARRAY_REF indices on the basis that: /* Index of the ARRAY_REF was zeroed in analyze_indices, thus we only need to check the stride and the lower bound of the reference. */ That was true back in 2007 when the code was added: static void dr_analyze_indices (struct data_reference *dr, struct loop *nest) { [...] while (handled_component_p (aref)) { if (TREE_CODE (aref) == ARRAY_REF) { op = TREE_OPERAND (aref, 1); access_fn = analyze_scalar_evolution (loop, op); access_fn = resolve_mixers (nest, access_fn); VEC_safe_push (tree, heap, access_fns, access_fn); TREE_OPERAND (aref, 1) = build_int_cst (TREE_TYPE (op), 0); } aref = TREE_OPERAND (aref, 0); } but the assignment was removed a few years ago. We were therefore treating "two->arr[i]" and "three->arr[i]" as loop invariant. Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. OK to install? Richard 2018-02-14 Richard Sandiford <richard.sandiford@linaro.org> gcc/ PR tree-optimization/84357 * tree-data-ref.c (object_address_invariant_in_loop_p): Check operand 1 of an ARRAY_REF too. gcc/testsuite/ PR tree-optimization/84357 * gcc.dg/vect/pr84357.c: New test.