Patchwork Skip gcc.dg/tree-ssa/isolate-*.c for AVR Target

login
register
mail settings
Submitter K_s, Vishnu
Date March 28, 2014, 10:16 a.m.
Message ID <512195FE6C78B243B073F23FF584A73361F93A4D@penmbx01>
Download mbox | patch
Permalink /patch/334639/
State New
Headers show

Comments

K_s, Vishnu - March 28, 2014, 10:16 a.m.
Hi all,

The tests added in gcc.dg/tree-ssa/isolate-*.c is failing for AVR target,
Because the isolate erroneous path pass needs -fdelete-null-pointer-checks
option to be enabled. For AVR target that option is disabled, this cause 
the tests to fail. Following Patch skip the isolate-* tests if 
"keeps_null_pointer_checks" is true. 

2014-03-28  Vishnu K S <Vishnu.k_s@atmel.com >

	* gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c: Skip test for AVR 
	* gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c: Ditto 
	* gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c: Ditto
	* gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c: Ditto
	* gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c: Ditto
Mike Stump - March 28, 2014, 7:04 p.m.
On Mar 28, 2014, at 3:16 AM, K_s, Vishnu <Vishnu.K_s@atmel.com> wrote:
> The tests added in gcc.dg/tree-ssa/isolate-*.c is failing for AVR target,
> Because the isolate erroneous path pass needs -fdelete-null-pointer-checks
> option to be enabled. For AVR target that option is disabled, this cause 
> the tests to fail. Following Patch skip the isolate-* tests if 
> "keeps_null_pointer_checks" is true. 

So I didn’t see a checkin, and I don’t see an Ok?  Each patch should have one or the other…  Without an Ok? I assume it’s been checked in, with an Ok? I take that as a review request.

I’ll assume you forgot the Ok?, Ok.

Since the AVR people are fairly active, I’ll let them check it in; gives them the opportunity to further consider it.

> 2014-03-28  Vishnu K S <Vishnu.k_s@atmel.com >
> 
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c: Skip test for AVR 
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c: Ditto 
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c: Ditto
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c: Ditto
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c: Ditto
Mike Stump - March 28, 2014, 7:21 p.m.
On Mar 28, 2014, at 12:04 PM, Mike Stump <mikestump@comcast.net> wrote:
> 
>> 2014-03-28  Vishnu K S <Vishnu.k_s@atmel.com >
>> 
>> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c: Skip test for AVR 
>> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c: Ditto 
>> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c: Ditto
>> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c: Ditto
>> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c: Ditto

So, no gcc/testsuite/ in the log,, no space before >, and two spaces after the name before the <, and end sentences with a ..
Jeff Law - April 4, 2014, 3:37 p.m.
On 03/28/14 04:16, K_s, Vishnu wrote:
> Hi all,
>
> The tests added in gcc.dg/tree-ssa/isolate-*.c is failing for AVR target,
> Because the isolate erroneous path pass needs -fdelete-null-pointer-checks
> option to be enabled. For AVR target that option is disabled, this cause
> the tests to fail. Following Patch skip the isolate-* tests if
> "keeps_null_pointer_checks" is true.
>
> 2014-03-28  Vishnu K S <Vishnu.k_s@atmel.com >
>
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c: Skip test for AVR
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c: Ditto
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c: Ditto
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c: Ditto
> 	* gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c: Ditto
This is fine for the trunk.  Please go ahead and install.

However, we generally discourage ports from turning off passes like this 
and particularly so without a comment as to why a pass is turned off.

That code was added to the AVR port here:

http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01968.html


If you could add a comment to the AVR port indicating that 
delete-null-pointer-checks is disabled because the hardware does not 
fault on a NULL dereference, it would be greatly appreciated.  Consider 
that comment addition pre-approved, just post it to the list for 
archival purposes.

Thanks,
Jeff
Richard Guenther - April 5, 2014, 9:45 a.m.
On Fri, Apr 4, 2014 at 5:37 PM, Jeff Law <law@redhat.com> wrote:
> On 03/28/14 04:16, K_s, Vishnu wrote:
>>
>> Hi all,
>>
>> The tests added in gcc.dg/tree-ssa/isolate-*.c is failing for AVR target,
>> Because the isolate erroneous path pass needs -fdelete-null-pointer-checks
>> option to be enabled. For AVR target that option is disabled, this cause
>> the tests to fail. Following Patch skip the isolate-* tests if
>> "keeps_null_pointer_checks" is true.
>>
>> 2014-03-28  Vishnu K S <Vishnu.k_s@atmel.com >
>>
>>         * gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c: Skip test for AVR
>>         * gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c: Ditto
>>         * gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c: Ditto
>>         * gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c: Ditto
>>         * gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c: Ditto
>
> This is fine for the trunk.  Please go ahead and install.
>
> However, we generally discourage ports from turning off passes like this and
> particularly so without a comment as to why a pass is turned off.
>
> That code was added to the AVR port here:
>
> http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01968.html
>
>
> If you could add a comment to the AVR port indicating that
> delete-null-pointer-checks is disabled because the hardware does not fault
> on a NULL dereference, it would be greatly appreciated.  Consider that
> comment addition pre-approved, just post it to the list for archival
> purposes.

Note that it is recommended (and documented) practice to treat
-fdelete-null-pointer-checks that way for targets that support objects
starting at 0.

Richard.

> Thanks,
> Jeff
>
>

Patch

--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-1.c
@@ -1,6 +1,7 @@ 

 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-isolate-paths" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */


 struct demangle_component
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c
index bfcaa2b..912d98e 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-2.c
@@ -1,5 +1,6 @@ 
 /* { dg-do compile } */
 /* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */


 int z;
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c
index 7dddd80..9c2c5d5 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-3.c
@@ -1,5 +1,6 @@ 
 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-isolate-paths" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */


 typedef long unsigned int size_t;
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c
index c9c074d..d50a2b2 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-4.c
@@ -1,5 +1,6 @@ 
 /* { dg-do compile } */
 /* { dg-options "-O2 -fisolate-erroneous-paths-attribute -fdump-tree-isolate-paths -fdump-tree-phicprop1" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */


 extern void foo(void *) __attribute__ ((__nonnull__ (1)));
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
index 4d01d5c..e6ae37a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/isolate-5.c
@@ -1,5 +1,6 @@ 
 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-isolate-paths -fdump-tree-optimized" } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */

Regards,
Vishnu KS