Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
Jakub Jelinek wrote:
On Fri, Jan 13, 2012 at 07:40:59PM +0100, Georg-Johann Lay wrote:
The ilp32 is the closes match:
The source casts pointer to int, int to pointer, long to int, uses 32-bit
initializers for int, assumes size_t is unsigned long any maybe others.
No. The source is just fine for any target where sizeof (long) == sizeof
(void *).
So both ilp32 and lp64.
Now just factored out avr. There is no dg-requite to filter out long!=void*, or
did I miss something?
Ok to apply?
Johann
* gcc.dg/lto/20091013-1_1.c: xfail for avr.
* gcc.dg/lto/20091013-1_2.c: xfail for avr.
Index: gcc.dg/lto/20091013-1_1.c
===
--- gcc.dg/lto/20091013-1_1.c (revision 183472)
+++ gcc.dg/lto/20091013-1_1.c (working copy)
@@ -1,3 +1,4 @@
+/* { dg-xfail-if cast to pointer of different size { avr-*-* } { * } {
} } */
typedef struct HDC__ { int unused; } *HDC;
typedef struct HFONT__ { int unused; } *HFONT;
Index: gcc.dg/lto/20091013-1_2.c
===
--- gcc.dg/lto/20091013-1_2.c (revision 183472)
+++ gcc.dg/lto/20091013-1_2.c (working copy)
@@ -1,3 +1,4 @@
+/* { dg-xfail-if cast to pointer of different size { avr-*-* } { * } {
} } */
typedef struct HDC__ { int unused; } *HDC;
typedef struct HFONT__ { int unused; } *HFONT;
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
On 01/13/2012 08:21 PM, Georg-Johann Lay wrote: I really don't expect anyone to run test suites on any platform or target supported by GCC. Such an approach is completely useless because of the amount of time and system resources and I never proposed that. But it appears that many of the test that are exposed to all targets are just dropped at the testsuite without even looking at them with respect to platform requirements. The most prominent of these requirements is sizeof(int) = 4 because the most PRs are reported for such platforms. Still I think that patches that are applied to test suite should experience some review. Anything else will just increase annoyance and frustration of developers that work for platforms that are not in the center of bolide hardware. For many test cases it's already sufficient to cut down constants so that they fit into 16 bits, and if actually 32-bit variable is needed GCC provides magic ike __INT32_TYPE__. I suggest listing these common testcase pitfalls / best practices somewhere, like for example, on a gcc wiki page. It's then easier to point people at the issues. -- Pedro Alves
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
Rainer Orth schrieb: Mike Stump mikest...@comcast.net writes: On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote: This test case is obviously written for 32-bit platforms, thus added dg-require-effective-target ilp32 to ensure that the pointer mess won't lead to FAILs because of warning: cast to pointer from integer of different size and warning: cast from pointer to integer of different size Ok to apply? Ok. * gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32. I wonder if the fix is right, though: the testcase currenly passes for 64-bit multilibs, but won't run there any longer if the patch is applied. Rainer What about int32plus instead of ilp32? That's still not exactly what the test case needs but closer than ilp32. Johann
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote: This test case is obviously written for 32-bit platforms, thus added dg-require-effective-target ilp32 to ensure that the pointer mess won't lead to FAILs because of warning: cast to pointer from integer of different size and warning: cast from pointer to integer of different size Ok to apply? Ok. * gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32.
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
Mike Stump mikest...@comcast.net writes: On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote: This test case is obviously written for 32-bit platforms, thus added dg-require-effective-target ilp32 to ensure that the pointer mess won't lead to FAILs because of warning: cast to pointer from integer of different size and warning: cast from pointer to integer of different size Ok to apply? Ok. * gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32. I wonder if the fix is right, though: the testcase currenly passes for 64-bit multilibs, but won't run there any longer if the patch is applied. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
On Jan 13, 2012, at 9:23 AM, Rainer Orth wrote: Mike Stump mikest...@comcast.net writes: On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote: This test case is obviously written for 32-bit platforms, thus added dg-require-effective-target ilp32 to ensure that the pointer mess won't lead to FAILs because of warning: cast to pointer from integer of different size and warning: cast from pointer to integer of different size Ok to apply? Ok. * gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32. I wonder if the fix is right, though: the testcase currenly passes for 64-bit multilibs, but won't run there any longer if the patch is applied. I rely upon words like obviously written for 32-bit platforms a little too much apparently, sorry. I agree, let's find some other solution. I've starred at the code, and is was not clear to me why you thought it was obviously for 32-bit platforms. So, I'd propose the -Wno- option silence the warning... Seem reasonable to people, if so, Ok.
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
Mike Stump wrote: On Jan 13, 2012, at 9:23 AM, Rainer Orth wrote: Mike Stump mikest...@comcast.net writes: On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote: This test case is obviously written for 32-bit platforms, thus added dg-require-effective-target ilp32 to ensure that the pointer mess won't lead to FAILs because of warning: cast to pointer from integer of different size and warning: cast from pointer to integer of different size Ok to apply? Ok. * gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32. I wonder if the fix is right, though: the testcase currenly passes for 64-bit multilibs, but won't run there any longer if the patch is applied. I rely upon words like obviously written for 32-bit platforms a little too much apparently, sorry. I agree, let's find some other solution. I've starred at the code, and is was not clear to me why you thought it was obviously for 32-bit platforms. So, I'd propose the -Wno- option silence the warning... Seem reasonable to people, if so, Ok. The ilp32 is the closes match: The source casts pointer to int, int to pointer, long to int, uses 32-bit initializers for int, assumes size_t is unsigned long any maybe others. Test cases like these are a permanent annoyance. There are hundreds of test cases that are not properly written or reviewed or gated by dg-require-foo. I wonder why such test cases go into the test suite in the first place if the come with such bunch of implications. Most of the test cases run on *all* targets that GCC claims to support. Johann
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
On Fri, Jan 13, 2012 at 07:40:59PM +0100, Georg-Johann Lay wrote: The ilp32 is the closes match: The source casts pointer to int, int to pointer, long to int, uses 32-bit initializers for int, assumes size_t is unsigned long any maybe others. No. The source is just fine for any target where sizeof (long) == sizeof (void *). So both ilp32 and lp64. I wonder why such test cases go into the test suite in the first place if the come with such bunch of implications. Because they test actual problems that were reported and usually fixed with the same commit, to avoid regressing in the future? Jakub
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
Jakub Jelinek wrote: On Fri, Jan 13, 2012 at 07:40:59PM +0100, Georg-Johann Lay wrote: The ilp32 is the closes match: The source casts pointer to int, int to pointer, long to int, uses 32-bit initializers for int, assumes size_t is unsigned long any maybe others. No. The source is just fine for any target where sizeof (long) == sizeof (void *). So both ilp32 and lp64. I wonder why such test cases go into the test suite in the first place if the come with such bunch of implications. Because they test actual problems that were reported and usually fixed with the same commit, to avoid regressing in the future? Jakub So it's fine to dump any code to the test suite no matter on what platform it might break or work? I.e. it is sufficient that it runs on at least one platform that complies to the C standard (for C test case)? Are there really no policies for test case that they behave reasonably? If, for example, there were hundreds of testcases in the test suites that break if sizeof(void) != 2 adding to the fun working with your platform: What would you think? Johann
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
Jakub Jelinek wrote: On Fri, Jan 13, 2012 at 08:25:39PM +0100, Georg-Johann Lay wrote: So it's fine to dump any code to the test suite no matter on what platform it might break or work? It is wrong to knowingly commit testcases that break on such platforms, but you really shouldn't expect every committer to test all testsuite additions on all targets. Jakub I really don't expect anyone to run test suites on any platform or target supported by GCC. Such an approach is completely useless because of the amount of time and system resources and I never proposed that. But it appears that many of the test that are exposed to all targets are just dropped at the testsuite without even looking at them with respect to platform requirements. The most prominent of these requirements is sizeof(int) = 4 because the most PRs are reported for such platforms. Still I think that patches that are applied to test suite should experience some review. Anything else will just increase annoyance and frustration of developers that work for platforms that are not in the center of bolide hardware. For many test cases it's already sufficient to cut down constants so that they fit into 16 bits, and if actually 32-bit variable is needed GCC provides magic like __INT32_TYPE__. Johann
Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*
On Jan 13, 2012, at 11:25 AM, Georg-Johann Lay wrote: So it's fine to dump any code to the test suite no matter on what platform it might break or work? No. It isn't fine, but it does happen. Some people spend a lot of time, trying to get the testcases minimal, portable and correct. Other people, can't even be bothered to test the testcase to ensure to actually even tested the failure. :-( The best way forward IMHO, is to not just fix it, but to provide feedback to those that author such testcases, to see if you can sensitize people to some of the issues. Some issues, like, ints are 16 bits, you're going to loose some or most people on most of the time. Other issues, should be easier. Timely feedback I think is the key though, best would be automated testing that can provide feedback within the hour, failing that, nightly.
