Re: [Patch, testsuite] Skip addr_equal-1 if target keeps null pointer checks

[Jan Hubicka <hubicka@ucw.cz>]

> On 09/29/2015 12:41 AM, Senthil Kumar Selvaraj wrote:
> >On Mon, Sep 28, 2015 at 01:38:18PM -0600, Jeff Law wrote:
> >>On 09/28/2015 02:15 AM, Senthil Kumar Selvaraj wrote:
> >>>Hi,
> >>>
> >>>   The below patch skips gcc.dg/addr_equal-1.c if the target keeps null
> >>>   pointer checks.
> >>>
> >>>   The test fails for such targets (avr, in my case) because the address
> >>>   comparison in the below code does not resolve to a constant, causing
> >>>   builtin_constant_p to return false and fail the test.
> >>>
> >>>   /* Variables and functions do not share same memory locations otherwise.  */
> >>>   if (!__builtin_constant_p ((void *)undef_fn0 == (void *)&undef_var0))
> >>>     abort ();
> >>>
> >>>   For targets that delete null pointer checks, the equality comparison expression
> >>>   is optimized away to 0, as the code in match.pd knows they can only be
> >>>   equal if they are both NULL, which cannot be true since
> >>>   flag-delete-null-pointer-checks is on.
> >>>
> >>>   For targets that keep null pointer checks, 0 is a valid address and the
> >>>	comparison expression is left as is, and that causes a later pass to
> >>>	fold the builtin_constant_p to a false value, resulting in the test failure.
> >>This sounds like a failing in the compiler itself, not a testsuite issue.
> >>
> >>Even on a target where objects can be at address 0, you can't have a
> >>variable and a function at the same address.
> >
> >Hmm, symtab_node::equal_address_to, which is where the address equality
> >check happens, has a comment that contradicts
> >your statement, and the function variable overlap check is done after the
> >NULL possibility check. The current code looks like this
> >
> >    /* If both symbols may resolve to NULL, we can not really prove them different.  */
> >     if (!nonzero_address () && !s2->nonzero_address ())
> >       return 2;
> >
> >     /* Except for NULL, functions and variables never overlap.  */
> >     if (TREE_CODE (decl) != TREE_CODE (s2->decl))
> >       return 0;
> >
> >Does anyone know why?
> The only case I could think of would be weak symbols.

Yep, the check is there for weak symbols.  nonzero_address returns true for most
common symbols.
I tried to be simply conservative here about correctness, but I assume we would have
non-transitive equivalence because something like this would trigger abort

if (fn == NULL && var == NULL)
  assert (fn == var);

I assume one can before nonzero_address check something like

 if (TREE_CODE (decl) != TREE_CODE (s2->decl)
     && ((analyzed && DECL_EXTERNAL (decl)) || !DECL_WEAK (decl))
     && ((s2->analyzed && DECL_EXTERNAL (s2->decl)) || !DECL_WEAK (decl)))
   return 0;

before nonzero_address check as I see that if both fn and var are defined
they can't bind to same address. (basically the second part of conditional
copy nonzero_address with flag_delete_null_pointer_checks assumed to be true,
extra parameter to nonzero_address may do)

Honza
> 
> jeff