On Mon, 4 Nov 2013, Richard Biener wrote:

On Mon, Nov 4, 2013 at 12:18 PM, Marc Glisse <marc.gli...@inria.fr> wrote:
On Mon, 4 Nov 2013, Richard Biener wrote:

On Mon, Nov 4, 2013 at 11:55 AM, Richard Biener
<richard.guent...@gmail.com> wrote:

On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse <marc.gli...@inria.fr>
wrote:

Hello,

the issue was described in the PR and the message linked from there.
ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may
detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size
never learns of it and uses the offset+size as if they were meaningful.


Well...  it certainly learns of it, but it chooses to ignore...

   if (TREE_CODE (ptr) == ADDR_EXPR)
-    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
-                                        &ref->offset, &t1, &t2);
+    {
+      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr,
0),
+                                                  &t, 0, &safe);
+      ref->offset = BITS_PER_UNIT * t;
+    }

safe == (t1 != -1 && t1 == t2)

note that ao_ref_init_from_ptr_and_size gets the size fed in as argument
so I fail to see why it matters at all ...?  That is, if you feed in a
wrong
size then it's the callers error.


I think one issue is that the above code uses get_ref_base_and_extent
on an address.  It should have used get_addr_base_and_unit_offset.


Isn't that what my patch does? Except that get_addr_base_and_unit_offset
often gives up and returns NULL_TREE, whereas I believe we still want a base
even if we have trouble determining a constant offset, so I modified
get_addr_base_and_unit_offset_1 a bit.

(not saying the result is pretty...)

I don't think we want get_addr_base_and_unit_offset to return non-NULL
for a non-constant offset.  But yes, inside your patch is the correct fix,
so if you drop changing get_addr_base_and_unit_offset ...

That means that in a number of cases, ao_ref_init_from_ptr_and_size will produce a meaningless ao_ref (both .ref and .base are 0). I expect that will cause regressions in code quality at least. Or did you mean something else?

get_inner_reference might be an option too (we have quite a few functions doing similar things).

--
Marc Glisse

Reply via email to