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

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

[Senthil Kumar Selvaraj]

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.

  If this is ok, could someone commit please? I don't have commit
  access.

Regards
Senthil

gcc/testsuite/ChangeLog

2015-09-28  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	* gcc.dg/addr_equal-1.c: Skip test if target keeps
	null pointer checks.

diff --git gcc/testsuite/gcc.dg/addr_equal-1.c gcc/testsuite/gcc.dg/addr_equal-1.c
index 94499f0..957b03a 100644
--- gcc/testsuite/gcc.dg/addr_equal-1.c
+++ gcc/testsuite/gcc.dg/addr_equal-1.c
@@ -3,6 +3,7 @@
 /* { dg-require-weak "" } */
 /* { dg-require-alias "" } */
 /* { dg-options "-O2" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
 void abort (void);
 extern int undef_var0, undef_var1;
 extern __attribute__ ((weak)) int weak_undef_var0;

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

[Mike Stump]

On Sep 28, 2015, at 1:15 AM, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote:
>  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.
> 
>  If this is ok, could someone commit please? I don't have commit
>  access.

So, my preference would be for the target maintainer (or a keeps_null_pointer_checks person) to review, if less then trivial.  Seem fine to me, but they should get first crack at the review.

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

[Jeff Law]

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.

Jeff

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

[Senthil Kumar Selvaraj]

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?

Regards
Senthil

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

[Jeff Law]

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.

jeff

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

[Jan Hubicka]

> 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