Re: [RFA][PATCH] Minor fix to aliasing machinery

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Oct 29, 2013 at 11:23 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Tue, 29 Oct 2013, Jeff Law wrote:
>
>> Marc pointed out that the handling of various BUILT_IN_MEM* and
>> BUILT_IN_STR* functions in tree-ssa-alias.c probably wasn't working as
>> intended because the code wasn't prepared for a common return value from
>> ao_ref_base, particularly returns of MEM_REFs.
>
>
> Hmm, ao_ref_base is never a pointer, so I'd say the issue is really with
> trying to use the SSA_NAME directly.

Yes.  Note that the code tries to relate two pointers but one is a
memory-reference (ao_ref_base) and one is either a SSA name pointer
or the result of get_addr_base_and_unit_offset (a memory reference as well).

>> This patch fixes the code to handle the trivial case of returning a
>> MEM_REF and adds a simple testcase.  There's probably a lot more that could
>> be done here.
>
>
> Thanks.
>
> I am not sure we want to keep the variable "base" that is either a decl/ref
> (from get_addr_base_and_unit_offset) or a pointer (dest). We know which case
> is which, but then forget it by storing both into base. Maybe something like
> this would be more "type-safe".
>
>               bool same = false;
>               if (TREE_CODE (dest) == ADDR_EXPR)
>                 same = (ref_base == get_addr_base_and_unit_offset
>                                       (TREE_OPERAND (dest, 0), &offset));
>               else if (TREE_CODE (dest) == SSA_NAME
>                        && TREE_CODE (ref_base) == MEM_REF)

   && integer_zerop (TREE_OPERAND (ref_base, 1))

>                 same = (TREE_OPERAND (ref_base, 0) == dest);
>               if (same)
>                 ...

Btw, get_addr_base_and_unit_offset may also return an offsetted
MEM_REF (from &MEM [p_3, 17] for example).  As we are interested in
pointers this could be handled by not requiring a memory reference
but extracting the base address and offset, covering more cases.

> By the way, I think the patch is fine as is, I am only discussing possible
> follow-ups.

Only slightly wrong-codish ;)

Richard.

> (see http://gcc.gnu.org/ml/gcc-patches/2013-10/txto0PQEYpiuz.txt for another
> approach using ao_ref_init_from_ptr_and_size)
>
> --
> Marc Glisse