Message ID | 4F8E7B61.7030701@st.com |
---|---|
State | New |
Headers | show |
On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote: > > Is it OK for trunk, bootstrapped and regtested on x86 I think Joseph Myers is on vacation, and there are no other C FE reviewers, but since this is c-common and convert.c, perhaps Jason and/or Richard can review it? Thanks, Manuel.
On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote: > On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote: >> >> Is it OK for trunk, bootstrapped and regtested on x86 > > I think Joseph Myers is on vacation, and there are no other C FE > reviewers, but since this is c-common and convert.c, perhaps Jason > and/or Richard can review it? The patch is ok if you put the PR52283 properly into a separate testcase, not by amending gcc.dg/case-const-2.c. Thanks, Richard. > Thanks, > > Manuel.
On 04/18/2012 11:51 AM, Richard Guenther wrote: > On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez > <lopezibanez@gmail.com> wrote: >> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote: >>> >>> Is it OK for trunk, bootstrapped and regtested on x86 >> >> I think Joseph Myers is on vacation, and there are no other C FE >> reviewers, but since this is c-common and convert.c, perhaps Jason >> and/or Richard can review it? > > The patch is ok if you put the PR52283 properly into a separate testcase, > not by amending gcc.dg/case-const-2.c. > Thanks, done at rev #186586. with this change. > Thanks, > Richard. > >> Thanks, >> >> Manuel.
On 19 April 2012 11:11, Christian Bruel <christian.bruel@st.com> wrote: > > > On 04/18/2012 11:51 AM, Richard Guenther wrote: >> On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez >> <lopezibanez@gmail.com> wrote: >>> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote: >>>> >>>> Is it OK for trunk, bootstrapped and regtested on x86 >>> >>> I think Joseph Myers is on vacation, and there are no other C FE >>> reviewers, but since this is c-common and convert.c, perhaps Jason >>> and/or Richard can review it? >> >> The patch is ok if you put the PR52283 properly into a separate testcase, >> not by amending gcc.dg/case-const-2.c. >> > > Thanks, done at rev #186586. with this change. Great! Just a minor nit, for future patches. There is the unwritten rule of adding the Changelogs to the commit log, like follows: 2012-04-19 Christian Bruel <christian.bruel@st.com> Manuel López-Ibáñez <manu@gcc.gnu.org> PR c/52283 PR c/37985 * stmt.c (warn_if_unused_value): Skip NOP_EXPR. * convert.c (convert_to_integer): Don't set TREE_NO_WARNING. testsuite/ * gcc.dg/pr52283.c: New test. * gcc.dg/pr37985.c: New test. Also, if you add to the commit logs the PR numbers like: PR whatever/NUMBER PR whatever2/NUMBER then bugzilla will show the commit on each PR. Cheers, Manuel.
On Thu, Apr 19, 2012 at 3:17 AM, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote: > On 19 April 2012 11:11, Christian Bruel <christian.bruel@st.com> wrote: >> >> >> On 04/18/2012 11:51 AM, Richard Guenther wrote: >>> On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez >>> <lopezibanez@gmail.com> wrote: >>>> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> wrote: >>>>> >>>>> Is it OK for trunk, bootstrapped and regtested on x86 >>>> >>>> I think Joseph Myers is on vacation, and there are no other C FE >>>> reviewers, but since this is c-common and convert.c, perhaps Jason >>>> and/or Richard can review it? >>> >>> The patch is ok if you put the PR52283 properly into a separate testcase, >>> not by amending gcc.dg/case-const-2.c. >>> >> >> Thanks, done at rev #186586. with this change. > > Great! > > Just a minor nit, for future patches. There is the unwritten rule of > adding the Changelogs to the commit log, like follows: > > 2012-04-19 Christian Bruel <christian.bruel@st.com> > Manuel López-Ibáñez <manu@gcc.gnu.org> > > PR c/52283 > PR c/37985 > * stmt.c (warn_if_unused_value): Skip NOP_EXPR. > * convert.c (convert_to_integer): Don't set TREE_NO_WARNING. > testsuite/ > * gcc.dg/pr52283.c: New test. > * gcc.dg/pr37985.c: New test. > > gcc.dg/pr52283.c failed on Linux/x86: FAIL: gcc.dg/pr52283.c (test for excess errors)
On Fri, Apr 20, 2012 at 5:23 PM, Greta Yorsh <Greta.Yorsh@arm.com> wrote: > Here is a patch to fix the failing test gcc.dg/pr52283.c. > Adding the missing dg-warning and dg-options. > > OK? Ok. Thanks, Richard. > > gcc/testsuite/ChangeLog > > 2012-04-20 Greta Yorsh <Greta.Yorsh@arm.com> > > * gcc.dg/pr52283.c: Add missing dg-warning and dg-options. > > > diff --git a/gcc/testsuite/gcc.dg/pr52283.c b/gcc/testsuite/gcc.dg/pr52283.c > index 33785a5..070e71a 100644 > --- a/gcc/testsuite/gcc.dg/pr52283.c > +++ b/gcc/testsuite/gcc.dg/pr52283.c > @@ -1,6 +1,7 @@ > /* Test for case labels not integer constant expressions but folding > to integer constants (used in Linux kernel). */ > /* { dg-do compile } */ > +/* { dg-options "-pedantic" } */ > > extern unsigned int u; > > @@ -9,7 +10,7 @@ b (int c) > { > switch (c) > { > - case (int) (2 | ((4 < 8) ? 8 : u)): > + case (int) (2 | ((4 < 8) ? 8 : u)): /* { dg-warning "case label is not > an integer constant expression" } */ > ; > } > } > >> -----Original Message----- >> From: H.J. Lu [mailto:hjl.tools@gmail.com] >> Sent: 19 April 2012 15:32 >> To: Manuel López-Ibáñez >> Cc: Christian Bruel; Richard Guenther; gcc-patches@gcc.gnu.org; Joseph >> S. Myers; Jason Merrill >> Subject: Re: PING: [PATCH] Fix PRs c/52283/37985 >> >> On Thu, Apr 19, 2012 at 3:17 AM, Manuel López-Ibáñez >> <lopezibanez@gmail.com> wrote: >> > On 19 April 2012 11:11, Christian Bruel <christian.bruel@st.com> >> wrote: >> >> >> >> >> >> On 04/18/2012 11:51 AM, Richard Guenther wrote: >> >>> On Wed, Apr 18, 2012 at 11:06 AM, Manuel López-Ibáñez >> >>> <lopezibanez@gmail.com> wrote: >> >>>> On 18 April 2012 10:29, Christian Bruel <christian.bruel@st.com> >> wrote: >> >>>>> >> >>>>> Is it OK for trunk, bootstrapped and regtested on x86 >> >>>> >> >>>> I think Joseph Myers is on vacation, and there are no other C FE >> >>>> reviewers, but since this is c-common and convert.c, perhaps Jason >> >>>> and/or Richard can review it? >> >>> >> >>> The patch is ok if you put the PR52283 properly into a separate >> testcase, >> >>> not by amending gcc.dg/case-const-2.c. >> >>> >> >> >> >> Thanks, done at rev #186586. with this change. >> > >> > Great! >> > >> > Just a minor nit, for future patches. There is the unwritten rule of >> > adding the Changelogs to the commit log, like follows: >> > >> > 2012-04-19 Christian Bruel <christian.bruel@st.com> >> > Manuel López-Ibáñez <manu@gcc.gnu.org> >> > >> > PR c/52283 >> > PR c/37985 >> > * stmt.c (warn_if_unused_value): Skip NOP_EXPR. >> > * convert.c (convert_to_integer): Don't set TREE_NO_WARNING. >> > testsuite/ >> > * gcc.dg/pr52283.c: New test. >> > * gcc.dg/pr37985.c: New test. >> > >> > >> >> gcc.dg/pr52283.c failed on Linux/x86: >> >> FAIL: gcc.dg/pr52283.c (test for excess errors) >> >> -- >> H.J. > > >
gcc/testsuite/ChangeLog 2010-02-15 Christian Bruel <christian.bruel@st.com> * gcc.dg/case-const-1.c: Test constant expression. * gcc.dg/case-const-2.c: Likewise. * gcc.dg/case-const-3.c: Likewise. gcc/ChangeLog 2012-03-29 Manuel López-Ibáñez <manu@gcc.gnu.org> PR c/52283 * c-familly/c-common.c (warn_if_unused_value): Skip NOP_EXPR. * convert.c (convert_to_integer): Don't set TREE_NO_WARNING. Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 186524) +++ gcc/c-family/c-common.c (working copy) @@ -1692,6 +1692,7 @@ case SAVE_EXPR: case NON_LVALUE_EXPR: + case NOP_EXPR: exp = TREE_OPERAND (exp, 0); goto restart; Index: gcc/convert.c =================================================================== --- gcc/convert.c (revision 186524) +++ gcc/convert.c (working copy) @@ -537,7 +537,6 @@ else if (outprec >= inprec) { enum tree_code code; - tree tem; /* If the precision of the EXPR's type is K bits and the destination mode has more bits, and the sign is changing, @@ -555,13 +554,7 @@ else code = NOP_EXPR; - tem = fold_unary (code, type, expr); - if (tem) - return tem; - - tem = build1 (code, type, expr); - TREE_NO_WARNING (tem) = 1; - return tem; + return fold_build1 (code, type, expr); } /* If TYPE is an enumeral type or a type with a precision less Index: gcc/testsuite/gcc.dg/pr37985.c =================================================================== --- gcc/testsuite/gcc.dg/pr37985.c (revision 0) +++ gcc/testsuite/gcc.dg/pr37985.c (revision 0) @@ -0,0 +1,8 @@ +/* PR c37985 */ +/* { dg-do compile } */ +/* { dg-options " -Wall -Wextra " } */ +unsigned char foo(unsigned char a) +{ + a >> 2; /* { dg-warning "no effect" } */ + return a; +} Index: gcc/testsuite/gcc.dg/case-const-1.c =================================================================== --- gcc/testsuite/gcc.dg/case-const-1.c (revision 186524) +++ gcc/testsuite/gcc.dg/case-const-1.c (working copy) @@ -1,9 +1,11 @@ /* Test for case labels not integer constant expressions but folding - to integer constants (used in Linux kernel, PR 39613). */ + to integer constants (used in Linux kernel, PR 39613, 52283). */ /* { dg-do compile } */ /* { dg-options "" } */ extern int i; +extern unsigned int u; + void f (int c) { @@ -13,3 +15,13 @@ ; } } + +void +b (int c) +{ + switch (c) + { + case (int) (2 | ((4 < 8) ? 8 : u)): + ; + } +} Index: gcc/testsuite/gcc.dg/case-const-2.c =================================================================== --- gcc/testsuite/gcc.dg/case-const-2.c (revision 186524) +++ gcc/testsuite/gcc.dg/case-const-2.c (working copy) @@ -1,9 +1,11 @@ /* Test for case labels not integer constant expressions but folding - to integer constants (used in Linux kernel, PR 39613). */ + to integer constants (used in Linux kernel, PR 39613, 52283). */ /* { dg-do compile } */ /* { dg-options "-pedantic" } */ extern int i; +extern unsigned int u; + void f (int c) { @@ -13,3 +15,14 @@ ; } } + +void +b (int c) +{ + switch (c) + { + case (int) (2 | ((4 < 8) ? 8 : u)): /* { dg-warning "case label is not an integer constant expression" } */ + ; + } +} + Index: gcc/testsuite/gcc.dg/case-const-3.c =================================================================== --- gcc/testsuite/gcc.dg/case-const-3.c (revision 186524) +++ gcc/testsuite/gcc.dg/case-const-3.c (working copy) @@ -1,9 +1,11 @@ /* Test for case labels not integer constant expressions but folding - to integer constants (used in Linux kernel, PR 39613). */ + to integer constants (used in Linux kernel, PR 39613, 52283, ). */ /* { dg-do compile } */ /* { dg-options "-pedantic-errors" } */ extern int i; +extern unsigned int u; + void f (int c) { @@ -13,3 +15,16 @@ ; } } + +void +b (int c) +{ + switch (c) + { + case (int) (2 | ((4 < 8) ? 8 : u)): /* { dg-error "case label is not an integer constant expression" } */ + ; + } +} + + +