Message ID | 20150928081559.GB14913@jaguar.corp.atmel.com |
---|---|
State | New |
Headers | show |
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.
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
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
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
> 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
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;