| 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;
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.