Re: [gimplefe] Regarding command line option handling
On Wed, May 4, 2016 at 8:41 AM, Prasad Ghangal wrote: > Hi ! > Currently I am trying to introduce new command line option -fgimple, > for that I am adding this to c.opt > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index 4f86876..88e55c6 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -66,6 +66,10 @@ C ObjC C++ ObjC++ Separate Alias(d) > -dump= > C ObjC C++ ObjC++ Joined Alias(d) > > +fgimple > +C Var(flag_gimple) Init(0) > +Enable parsing GIMPLE > + > -imacros > C ObjC C++ ObjC++ Separate Alias(imacros) MissingArgError(missing > filename after %qs) > > > But I am getting error as - "gcc: error: unrecognized command line > option ‘-fgimple ’; did you mean ‘-fgimple ’?" > > I am unable to find where to handle it. Did you properly re-build gcc after that change? It should work just fine. Richard. > > > Till now I am able to JUST parse __GIMPLE keyword > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index cae2faf..1ccb4d6 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -468,6 +468,7 @@ const struct c_common_resword c_common_reswords[] = >{ "__extension__",RID_EXTENSION,0 }, >{ "__func__",RID_C99_FUNCTION_NAME, 0 }, >{ "__has_nothrow_assign", RID_HAS_NOTHROW_ASSIGN, D_CXXONLY }, > + { "__GIMPLE",RID_GIMPLE,0 }, >{ "__has_nothrow_constructor", RID_HAS_NOTHROW_CONSTRUCTOR, D_CXXONLY }, >{ "__has_nothrow_copy", RID_HAS_NOTHROW_COPY, D_CXXONLY }, >{ "__has_trivial_assign", RID_HAS_TRIVIAL_ASSIGN, D_CXXONLY }, > @@ -12393,6 +12394,7 @@ keyword_is_function_specifier (enum rid keyword) > case RID_NORETURN: > case RID_VIRTUAL: > case RID_EXPLICIT: > +case RID_GIMPLE: >return true; > default: >return false; > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h > index 663e457..a91665f 100644 > --- a/gcc/c-family/c-common.h > +++ b/gcc/c-family/c-common.h > @@ -64,10 +64,10 @@ enum rid >/* Modifiers: */ >/* C, in empirical order of frequency. */ >RID_STATIC = 0, > - RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN, > - RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE, > - RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT, > - RID_NORETURN, RID_ATOMIC, > + RID_UNSIGNED, RID_LONG, RID_CONST, RID_EXTERN, > + RID_GIMPLE,RID_REGISTER, RID_TYPEDEF, RID_SHORT, > + RID_INLINE,RID_VOLATILE, RID_SIGNED, RID_AUTO, > + RID_RESTRICT, RID_NORETURN, RID_ATOMIC, > >/* C extensions */ >RID_COMPLEX, RID_THREAD, RID_SAT, > diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c > index f0c677b..e690ca3 100644 > --- a/gcc/c/c-decl.c > +++ b/gcc/c/c-decl.c > @@ -10401,6 +10401,8 @@ declspecs_add_scspec (source_location loc, > case RID_TYPEDEF: >n = csc_typedef; >break; > +case RID_GIMPLE: > + break; > default: >gcc_unreachable (); > } > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c > index bdd669d..266b672 100644 > --- a/gcc/c/c-parser.c > +++ b/gcc/c/c-parser.c > @@ -732,6 +732,7 @@ c_token_starts_declspecs (c_token *token) > case RID_ALIGNAS: > case RID_ATOMIC: > case RID_AUTO_TYPE: > +case RID_GIMPLE: >return true; > default: >if (token->keyword >= RID_FIRST_INT_N > @@ -2461,6 +2462,7 @@ c_parser_declspecs (c_parser *parser, struct > c_declspecs *specs, > case RID_NORETURN: > case RID_AUTO: > case RID_THREAD: > +case RID_GIMPLE: >if (!scspec_ok) > goto out; >attrs_ok = true; > @@ -3960,6 +3962,7 @@ c_parser_attribute_any_word (c_parser *parser) > case RID_INT_N_1: > case RID_INT_N_2: > case RID_INT_N_3: > +case RID_GIMPLE: >ok = true; >break; > default: > > > > After recognizing __GIMPLE, I am planning to call gimple_fn_parser > function which will parse gimple body. > For pass manager, basic idea is to introduce new member to struct > function as pass_list and modify function execute_pass_list to run > only passes in pass list. > > > > Thanks, > Prasad Ghangal
Re: [gimplefe] Regarding command line option handling
On 4 May 2016 at 13:02, Richard Biener wrote: > On Wed, May 4, 2016 at 8:41 AM, Prasad Ghangal > wrote: >> Hi ! >> Currently I am trying to introduce new command line option -fgimple, >> for that I am adding this to c.opt >> >> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt >> index 4f86876..88e55c6 100644 >> --- a/gcc/c-family/c.opt >> +++ b/gcc/c-family/c.opt >> @@ -66,6 +66,10 @@ C ObjC C++ ObjC++ Separate Alias(d) >> -dump= >> C ObjC C++ ObjC++ Joined Alias(d) >> >> +fgimple >> +C Var(flag_gimple) Init(0) >> +Enable parsing GIMPLE >> + >> -imacros >> C ObjC C++ ObjC++ Separate Alias(imacros) MissingArgError(missing >> filename after %qs) >> >> >> But I am getting error as - "gcc: error: unrecognized command line >> option ‘-fgimple ’; did you mean ‘-fgimple ’?" >> >> I am unable to find where to handle it. > > Did you properly re-build gcc after that change? It should work just fine. > > Richard. > Yes, I did stage 1 build on latest revision. Still it's giving this strange error - "gcc: error: unrecognized command line option ‘-fgimple’; did you mean ‘-fgimple’?" >> >> >> Till now I am able to JUST parse __GIMPLE keyword >> >> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >> index cae2faf..1ccb4d6 100644 >> --- a/gcc/c-family/c-common.c >> +++ b/gcc/c-family/c-common.c >> @@ -468,6 +468,7 @@ const struct c_common_resword c_common_reswords[] = >>{ "__extension__",RID_EXTENSION,0 }, >>{ "__func__",RID_C99_FUNCTION_NAME, 0 }, >>{ "__has_nothrow_assign", RID_HAS_NOTHROW_ASSIGN, D_CXXONLY }, >> + { "__GIMPLE",RID_GIMPLE,0 }, >>{ "__has_nothrow_constructor", RID_HAS_NOTHROW_CONSTRUCTOR, D_CXXONLY }, >>{ "__has_nothrow_copy", RID_HAS_NOTHROW_COPY, D_CXXONLY }, >>{ "__has_trivial_assign", RID_HAS_TRIVIAL_ASSIGN, D_CXXONLY }, >> @@ -12393,6 +12394,7 @@ keyword_is_function_specifier (enum rid keyword) >> case RID_NORETURN: >> case RID_VIRTUAL: >> case RID_EXPLICIT: >> +case RID_GIMPLE: >>return true; >> default: >>return false; >> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h >> index 663e457..a91665f 100644 >> --- a/gcc/c-family/c-common.h >> +++ b/gcc/c-family/c-common.h >> @@ -64,10 +64,10 @@ enum rid >>/* Modifiers: */ >>/* C, in empirical order of frequency. */ >>RID_STATIC = 0, >> - RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN, >> - RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE, >> - RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT, >> - RID_NORETURN, RID_ATOMIC, >> + RID_UNSIGNED, RID_LONG, RID_CONST, RID_EXTERN, >> + RID_GIMPLE,RID_REGISTER, RID_TYPEDEF, RID_SHORT, >> + RID_INLINE,RID_VOLATILE, RID_SIGNED, RID_AUTO, >> + RID_RESTRICT, RID_NORETURN, RID_ATOMIC, >> >>/* C extensions */ >>RID_COMPLEX, RID_THREAD, RID_SAT, >> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c >> index f0c677b..e690ca3 100644 >> --- a/gcc/c/c-decl.c >> +++ b/gcc/c/c-decl.c >> @@ -10401,6 +10401,8 @@ declspecs_add_scspec (source_location loc, >> case RID_TYPEDEF: >>n = csc_typedef; >>break; >> +case RID_GIMPLE: >> + break; >> default: >>gcc_unreachable (); >> } >> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c >> index bdd669d..266b672 100644 >> --- a/gcc/c/c-parser.c >> +++ b/gcc/c/c-parser.c >> @@ -732,6 +732,7 @@ c_token_starts_declspecs (c_token *token) >> case RID_ALIGNAS: >> case RID_ATOMIC: >> case RID_AUTO_TYPE: >> +case RID_GIMPLE: >>return true; >> default: >>if (token->keyword >= RID_FIRST_INT_N >> @@ -2461,6 +2462,7 @@ c_parser_declspecs (c_parser *parser, struct >> c_declspecs *specs, >> case RID_NORETURN: >> case RID_AUTO: >> case RID_THREAD: >> +case RID_GIMPLE: >>if (!scspec_ok) >> goto out; >>attrs_ok = true; >> @@ -3960,6 +3962,7 @@ c_parser_attribute_any_word (c_parser *parser) >> case RID_INT_N_1: >> case RID_INT_N_2: >> case RID_INT_N_3: >> +case RID_GIMPLE: >>ok = true; >>break; >> default: >> >> >> >> After recognizing __GIMPLE, I am planning to call gimple_fn_parser >> function which will parse gimple body. >> For pass manager, basic idea is to introduce new member to struct >> function as pass_list and modify function execute_pass_list to run >> only passes in pass list. >> >> >> >> Thanks, >> Prasad Ghangal
Re: [gimplefe] Regarding command line option handling
On Wed, May 4, 2016 at 11:46 AM, Prasad Ghangal wrote: > On 4 May 2016 at 13:02, Richard Biener wrote: >> On Wed, May 4, 2016 at 8:41 AM, Prasad Ghangal >> wrote: >>> Hi ! >>> Currently I am trying to introduce new command line option -fgimple, >>> for that I am adding this to c.opt >>> >>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt >>> index 4f86876..88e55c6 100644 >>> --- a/gcc/c-family/c.opt >>> +++ b/gcc/c-family/c.opt >>> @@ -66,6 +66,10 @@ C ObjC C++ ObjC++ Separate Alias(d) >>> -dump= >>> C ObjC C++ ObjC++ Joined Alias(d) >>> >>> +fgimple >>> +C Var(flag_gimple) Init(0) >>> +Enable parsing GIMPLE >>> + >>> -imacros >>> C ObjC C++ ObjC++ Separate Alias(imacros) MissingArgError(missing >>> filename after %qs) >>> >>> >>> But I am getting error as - "gcc: error: unrecognized command line >>> option ‘-fgimple ’; did you mean ‘-fgimple ’?" >>> >>> I am unable to find where to handle it. >> >> Did you properly re-build gcc after that change? It should work just fine. >> >> Richard. >> > > Yes, I did stage 1 build on latest revision. Still it's giving this > strange error - "gcc: error: unrecognized command line option > ‘-fgimple’; did you mean ‘-fgimple’?" The error is indeed strage. W/o the patch I get > ./xgcc -B. -fgimple -S t.i xgcc: error: unrecognized command line option '-fgimple'; did you mean '--compile'? and with it (re-building cc1 and xgcc inside gcc/ of my dev-tree) > ./xgcc -B. -fgimple -S t.i (no error) so it works fine for me. Note that 'gcc' inside the build tree is called xgcc. Richard. > >>> >>> >>> Till now I am able to JUST parse __GIMPLE keyword >>> >>> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >>> index cae2faf..1ccb4d6 100644 >>> --- a/gcc/c-family/c-common.c >>> +++ b/gcc/c-family/c-common.c >>> @@ -468,6 +468,7 @@ const struct c_common_resword c_common_reswords[] = >>>{ "__extension__",RID_EXTENSION,0 }, >>>{ "__func__",RID_C99_FUNCTION_NAME, 0 }, >>>{ "__has_nothrow_assign", RID_HAS_NOTHROW_ASSIGN, D_CXXONLY }, >>> + { "__GIMPLE",RID_GIMPLE,0 }, >>>{ "__has_nothrow_constructor", RID_HAS_NOTHROW_CONSTRUCTOR, D_CXXONLY }, >>>{ "__has_nothrow_copy", RID_HAS_NOTHROW_COPY, D_CXXONLY }, >>>{ "__has_trivial_assign", RID_HAS_TRIVIAL_ASSIGN, D_CXXONLY }, >>> @@ -12393,6 +12394,7 @@ keyword_is_function_specifier (enum rid keyword) >>> case RID_NORETURN: >>> case RID_VIRTUAL: >>> case RID_EXPLICIT: >>> +case RID_GIMPLE: >>>return true; >>> default: >>>return false; >>> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h >>> index 663e457..a91665f 100644 >>> --- a/gcc/c-family/c-common.h >>> +++ b/gcc/c-family/c-common.h >>> @@ -64,10 +64,10 @@ enum rid >>>/* Modifiers: */ >>>/* C, in empirical order of frequency. */ >>>RID_STATIC = 0, >>> - RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN, >>> - RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE, >>> - RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT, >>> - RID_NORETURN, RID_ATOMIC, >>> + RID_UNSIGNED, RID_LONG, RID_CONST, RID_EXTERN, >>> + RID_GIMPLE,RID_REGISTER, RID_TYPEDEF, RID_SHORT, >>> + RID_INLINE,RID_VOLATILE, RID_SIGNED, RID_AUTO, >>> + RID_RESTRICT, RID_NORETURN, RID_ATOMIC, >>> >>>/* C extensions */ >>>RID_COMPLEX, RID_THREAD, RID_SAT, >>> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c >>> index f0c677b..e690ca3 100644 >>> --- a/gcc/c/c-decl.c >>> +++ b/gcc/c/c-decl.c >>> @@ -10401,6 +10401,8 @@ declspecs_add_scspec (source_location loc, >>> case RID_TYPEDEF: >>>n = csc_typedef; >>>break; >>> +case RID_GIMPLE: >>> + break; >>> default: >>>gcc_unreachable (); >>> } >>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c >>> index bdd669d..266b672 100644 >>> --- a/gcc/c/c-parser.c >>> +++ b/gcc/c/c-parser.c >>> @@ -732,6 +732,7 @@ c_token_starts_declspecs (c_token *token) >>> case RID_ALIGNAS: >>> case RID_ATOMIC: >>> case RID_AUTO_TYPE: >>> +case RID_GIMPLE: >>>return true; >>> default: >>>if (token->keyword >= RID_FIRST_INT_N >>> @@ -2461,6 +2462,7 @@ c_parser_declspecs (c_parser *parser, struct >>> c_declspecs *specs, >>> case RID_NORETURN: >>> case RID_AUTO: >>> case RID_THREAD: >>> +case RID_GIMPLE: >>>if (!scspec_ok) >>> goto out; >>>attrs_ok = true; >>> @@ -3960,6 +3962,7 @@ c_parser_attribute_any_word (c_parser *parser) >>> case RID_INT_N_1: >>> case RID_INT_N_2: >>> case RID_INT_N_3: >>> +case RID_GIMPLE: >>>ok = true; >>>break; >>> default: >>> >>> >>> >>> After recognizing __GIMPLE, I am planning to call gimple_fn_parser >>> function which will parse gimple body. >>> For pass manager, basic idea is to introduce new member to struct >>> function as pass_list and modify function execute_pass_list to run >>> only passes in pass list. >>> >>> >>> >>> Thanks, >>> P
Re: increase alignment of global structs in increase_alignment pass
On 23 February 2016 at 21:49, Prathamesh Kulkarni wrote: > On 23 February 2016 at 17:31, Richard Biener wrote: >> On Tue, 23 Feb 2016, Prathamesh Kulkarni wrote: >> >>> On 22 February 2016 at 17:36, Richard Biener wrote: >>> > On Mon, 22 Feb 2016, Prathamesh Kulkarni wrote: >>> > >>> >> Hi Richard, >>> >> As discussed in private mail, this version of patch attempts to >>> >> increase alignment >>> >> of global struct decl if it contains an an array field(s) and array's >>> >> offset is a multiple of the alignment of vector type corresponding to >>> >> it's scalar type and recursively checks for nested structs. >>> >> eg: >>> >> static struct >>> >> { >>> >> int a, b, c, d; >>> >> int k[4]; >>> >> float f[10]; >>> >> }; >>> >> k is a candidate array since it's offset is 16 and alignment of >>> >> "vector (4) int" is 8. >>> >> Similarly for f. >>> >> >>> >> I haven't been able to create a test-case where there are >>> >> multiple candidate arrays and vector alignment of arrays are different. >>> >> I suppose in this case we will have to increase alignment >>> >> of the struct by the max alignment ? >>> >> eg: >>> >> static struct >>> >> { >>> >> >>> >> T1 k[S1] >>> >> >>> >> T2 f[S2] >>> >> >>> >> }; >>> >> >>> >> if V1 is vector type corresponding to T1, and V2 corresponding vector >>> >> type to T2, >>> >> offset (k) % align(V1) == 0 and offset (f) % align(V2) == 0 >>> >> and align (V1) > align(V2) then we will increase alignment of struct >>> >> by align(V1). >>> >> >>> >> Testing showed FAIL for g++.dg/torture/pr31863.C due to program timeout. >>> >> Initially it appeared to me, it went in infinite loop. However >>> >> on second thoughts I think it's probably not an infinite loop, rather >>> >> taking (extraordinarily) large amount of time >>> >> to compile the test-case with the patch. >>> >> The test-case builds quickly for only 2 instantiations of ClassSpec >>> >> (ClassSpec, >>> >> ClassSpec) >>> >> Building with 22 instantiations (upto ClassSpec) takes up >>> >> to ~1m to compile. >>> >> with: >>> >> 23 instantiations: ~2m >>> >> 24 instantiations: ~5m >>> >> For 30 instantiations I terminated cc1plus after 13m (by SIGKILL). >>> >> >>> >> I guess it shouldn't go in an infinite loop because: >>> >> a) structs cannot have circular references. >>> >> b) works for lower number of instantiations >>> >> However I have no sound evidence that it cannot be in infinite loop. >>> >> I don't understand why a decl node is getting visited more than once >>> >> for that test-case. >>> >> >>> >> Using a hash_map to store alignments of decl's so that decl node gets >>> >> visited >>> >> only once prevents the issue. >>> > >>> > Maybe aliases. Try not walking vnode->alias == true vars. >>> Hi, >>> I have a hypothesis why decl node gets visited multiple times. >>> >>> Consider the test-case: >>> template >>> struct X >>> { >>> T a; >>> virtual int foo() { return N; } >>> }; >>> >>> typedef struct X x_1; >>> typedef struct X x_2; >>> >>> static x_1 obj1 __attribute__((used)); >>> static x_2 obj2 __attribute__((used)); >>> >>> Two additional structs are created by C++FE, c++filt shows: >>> _ZTI1XIiLj1EE -> typeinfo for X >>> _ZTI1XIiLj2EE -> typeinfo for X >>> >>> Both of these structs have only one field D.2991 and it appears it's >>> *shared* between them: >>> struct D.2991; >>> const void * D.2980; >>> const char * D.2981; >>> >>> Hence the decl node D.2991 and it's fields (D.2890, D.2981) get visited >>> twice: >>> once when walking _ZTI1XIiLj1EE and 2nd time when walking _ZTI1XIiLj2EE >>> >>> Dump of walking over the global structs for above test-case: >>> http://pastebin.com/R5SABW0c >>> >>> So it appears to me to me a DAG (interior node == struct decl, leaf == >>> non-struct field, >>> edge from node1 -> node2 if node2 is field of node1) is getting >>> created when struct decl >>> is a type-info object. >>> >>> I am not really clear on how we should proceed: >>> a) Keep using hash_map to store alignments so that every decl gets >>> visited only once. >>> b) Skip walking artificial record decls. >>> I am not sure if skipping all artificial struct decls would be a good >>> idea, but I don't >>> think it's possible to identify if a struct-decl is typeinfo struct at >>> middle-end ? >> >> You shouldn't end up walking those when walking the type of >> global decls. That is, don't walk typeinfo decls - yes, practically >> that means just not walking DECL_ARTIFICIAL things. > Hi, > I have done the changes in the patch (attached) and cross-tested > on arm*-*-* and aarch64*-*-* without failures. > Is it OK for stage-1 ? Hi, Is the attached patch OK for trunk ? Bootstrapped and tested on aarch64-linux-gnu, ppc64le-linux-gnu. Cross-tested on arm*-*-* and aarch64*-*-*. Thanks, Prathamesh > > Thanks, > Prathamesh >> >> Richard. diff --git a/gcc/testsuite/gcc.dg/vect/section-anchors-vect-70.c b/gcc/testsuite/gcc.dg/vect/section-anchors-vect-70.c new file mode 100644 index 000..
Re: [gimplefe] Regarding command line option handling
On 4 May 2016 at 15:54, Richard Biener wrote: > On Wed, May 4, 2016 at 11:46 AM, Prasad Ghangal > wrote: >> On 4 May 2016 at 13:02, Richard Biener wrote: >>> On Wed, May 4, 2016 at 8:41 AM, Prasad Ghangal >>> wrote: Hi ! Currently I am trying to introduce new command line option -fgimple, for that I am adding this to c.opt diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 4f86876..88e55c6 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -66,6 +66,10 @@ C ObjC C++ ObjC++ Separate Alias(d) -dump= C ObjC C++ ObjC++ Joined Alias(d) +fgimple +C Var(flag_gimple) Init(0) +Enable parsing GIMPLE + -imacros C ObjC C++ ObjC++ Separate Alias(imacros) MissingArgError(missing filename after %qs) But I am getting error as - "gcc: error: unrecognized command line option ‘-fgimple ’; did you mean ‘-fgimple ’?" I am unable to find where to handle it. >>> >>> Did you properly re-build gcc after that change? It should work just fine. >>> >>> Richard. >>> >> >> Yes, I did stage 1 build on latest revision. Still it's giving this >> strange error - "gcc: error: unrecognized command line option >> ‘-fgimple’; did you mean ‘-fgimple’?" > > The error is indeed strage. W/o the patch I get > >> ./xgcc -B. -fgimple -S t.i > xgcc: error: unrecognized command line option '-fgimple'; did you mean > '--compile'? > > and with it (re-building cc1 and xgcc inside gcc/ of my dev-tree) > >> ./xgcc -B. -fgimple -S t.i > (no error) > > so it works fine for me. Note that 'gcc' inside the build tree is called > xgcc. > Yes, it works for $build/gcc/cc1 but not for $prefix/bin/gcc i.e. after installing > Richard. > > >> Till now I am able to JUST parse __GIMPLE keyword diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index cae2faf..1ccb4d6 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -468,6 +468,7 @@ const struct c_common_resword c_common_reswords[] = { "__extension__",RID_EXTENSION,0 }, { "__func__",RID_C99_FUNCTION_NAME, 0 }, { "__has_nothrow_assign", RID_HAS_NOTHROW_ASSIGN, D_CXXONLY }, + { "__GIMPLE",RID_GIMPLE,0 }, { "__has_nothrow_constructor", RID_HAS_NOTHROW_CONSTRUCTOR, D_CXXONLY }, { "__has_nothrow_copy", RID_HAS_NOTHROW_COPY, D_CXXONLY }, { "__has_trivial_assign", RID_HAS_TRIVIAL_ASSIGN, D_CXXONLY }, @@ -12393,6 +12394,7 @@ keyword_is_function_specifier (enum rid keyword) case RID_NORETURN: case RID_VIRTUAL: case RID_EXPLICIT: +case RID_GIMPLE: return true; default: return false; diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 663e457..a91665f 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -64,10 +64,10 @@ enum rid /* Modifiers: */ /* C, in empirical order of frequency. */ RID_STATIC = 0, - RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN, - RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE, - RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT, - RID_NORETURN, RID_ATOMIC, + RID_UNSIGNED, RID_LONG, RID_CONST, RID_EXTERN, + RID_GIMPLE,RID_REGISTER, RID_TYPEDEF, RID_SHORT, + RID_INLINE,RID_VOLATILE, RID_SIGNED, RID_AUTO, + RID_RESTRICT, RID_NORETURN, RID_ATOMIC, /* C extensions */ RID_COMPLEX, RID_THREAD, RID_SAT, diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index f0c677b..e690ca3 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -10401,6 +10401,8 @@ declspecs_add_scspec (source_location loc, case RID_TYPEDEF: n = csc_typedef; break; +case RID_GIMPLE: + break; default: gcc_unreachable (); } diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index bdd669d..266b672 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -732,6 +732,7 @@ c_token_starts_declspecs (c_token *token) case RID_ALIGNAS: case RID_ATOMIC: case RID_AUTO_TYPE: +case RID_GIMPLE: return true; default: if (token->keyword >= RID_FIRST_INT_N @@ -2461,6 +2462,7 @@ c_parser_declspecs (c_parser *parser, struct c_declspecs *specs, case RID_NORETURN: case RID_AUTO: case RID_THREAD: +case RID_GIMPLE: if (!scspec_ok) goto out; attrs_ok = true; @@ -3960,6 +3962,7 @@ c_parser_attribute_any_word (c_parser *parser) case RID_INT_N_1: case RID_INT_N_2: case RID_INT_N_3: +case RID_GIMPLE: ok = true; break; default: After recognizing __
Please, take '-Wmisleading-indentation' out of -Wall
[Please, CC me. I'm not subscribed to gcc@gcc.gnu.org]. First of all, thank you very much for gcc. I am not an expert in gcc. Please, forgive any mistakes in this message. :-) After compiling all my projects with gcc-6.1, I have received warnings related to -Wmisleading-indentation in four of them for constructs that I do not consider questionable. (Mainly one-liners inside switch statements with no indentation involved). As it is now, the name of the option is misleading, because -Wmisleading-indentation does not limit itself to warn about misleading indentation. One of the warnings is a false positive for the following code from GNU ed (the 'break' is aligned with the 'while', all spaces, no tabs, yet gcc complains that it is misleadingly indented): case '#': while( *(*ibufpp)++ != '\n' ) ; break; I consider -Wmisleading-indentation an useful addition to gcc, but IMO there are a number of reasons that make its inclusion in -Wall inappropriate or at least excessively annoying. To begin with, the C and C++ standards state clearly that the amount of whitespace characters separating tokens is not significant. Therefore, warning about indentation in C/C++ must be considered optional. More optional than -Woverlength-strings, which is not in -Wall nor in -Wextra. C/C++ allows creative use of whitespace to highlight the estructure of the code in ways that -Wmisleading-indentation can't anticipate. Including -Wmisleading-indentation in -Wall forces the developer to surrender such freedom just to placate gcc: http://www.gnu.org/prep/standards/html_node/Syntactic-Conventions.html "Don't make the program ugly just to placate static analysis tools such as 'lint', 'clang', and GCC with extra warnings options such as '-Wconversion' and '-Wundef'. These tools can help find bugs and unclear code, but they can also generate so many false alarms that it hurts readability to silence them with unnecessary casts, wrappers, and other complications. For example, please don't insert casts to 'void' or calls to do-nothing functions merely to pacify a lint checker". I also think that -Wmisleading-indentation should not be enabled by -Wall nor -Wextra because: - -Wall is defined in the gcc manual as enabling all the warnings about constructions that are easy to avoid (or modify to prevent the warning), even in conjunction with macros. This is not true of -Wmisleading-indentation. - It can't be portably disabled; older versions of gcc do not accept '-Wno-misleading-indentation'. (At least 4.1.2 does not accept it). - If there are no macros involved, it is independent of code generation; once tested in the developer's machine there is no need to test it again on every user's machine. - It makes compilation a 0.5-1% slower for every user for no reason. - -Wempty-body is much simpler to test for, and in general less questionable than -Wmisleading-indentation, yet it is not enabled by -Wall. I think that keeping separated categories of warnings (instead of warning about everything by default) is a valuable feature. Maybe both -Wempty-body and -Wmisleading-indentation (and any future similar options) could be put in a new category (-Wcoding-style or -Wstatic-analysis, for example). To summarize, I want 'gcc -Wall -Wextra' to: - not warn about the use of whitespace beyond what the standard mandates. - not warn about the format of one-liners. - not being slowed down because of indentation parsing. Please, take '-Wmisleading-indentation' out of -Wall (and don't move it to -Wextra). Thanks in advance, Antonio.
Re: Please, take '-Wmisleading-indentation' out of -Wall
On Wed, 2016-05-04 at 18:15 +0200, Antonio Diaz Diaz wrote: > [Please, CC me. I'm not subscribed to gcc@gcc.gnu.org]. > > First of all, thank you very much for gcc. > > I am not an expert in gcc. Please, forgive any mistakes in this > message. :-) > > After compiling all my projects with gcc-6.1, I have received > warnings > related to -Wmisleading-indentation in four of them for constructs > that > I do not consider questionable. (Mainly one-liners inside switch > statements with no indentation involved). As it is now, the name of > the > option is misleading, because -Wmisleading-indentation does not limit > itself to warn about misleading indentation. > > One of the warnings is a false positive for the following code from > GNU > ed (the 'break' is aligned with the 'while', all spaces, no tabs, yet > gcc complains that it is misleadingly indented): > case '#': while( *(*ibufpp)++ != '\n' ) ; >break; I agree that this is a false positive. I downloaded ed-1.13 and tried it with a recent build of gcc and I also see it, in main_loop.c. I've filed a bug about this here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70954 (aka "PR c/70954") and I'll have a look at fixing it. Sorry about the bug. Does that bug fully capture the kind of issue you're seeing, or are there other kinds of false positives? (I only see that single false positive in ed-1.13, but you mentioned four projects in total). > I consider -Wmisleading-indentation an useful addition to gcc, but > IMO > there are a number of reasons that make its inclusion in -Wall > inappropriate or at least excessively annoying. > > To begin with, the C and C++ standards state clearly that the amount > of > whitespace characters separating tokens is not significant. > Therefore, > warning about indentation in C/C++ must be considered optional. More > optional than -Woverlength-strings, which is not in -Wall > nor in -Wextra. > > C/C++ allows creative use of whitespace to highlight the estructure > of > the code in ways that -Wmisleading-indentation can't anticipate. Warning about indentation is gcc *is* optional, that's why you have to enable -Wall or -Wmisleading-indentation to get it. Re "-Woverlength-strings", I don't think that there's a single scale of "optionalness" against which warnings can be considered. > Including -Wmisleading-indentation in -Wall forces the developer to > surrender such freedom just to placate gcc: > > http://www.gnu.org/prep/standards/html_node/Syntactic-Conventions.htm > l > "Don't make the program ugly just to placate static analysis tools > such > as 'lint', 'clang', and GCC with extra warnings options such as > '-Wconversion' and '-Wundef'. These tools can help find bugs and > unclear > code, but they can also generate so many false alarms that it hurts > readability to silence them with unnecessary casts, wrappers, and > other > complications. For example, please don't insert casts to 'void' or > calls > to do-nothing functions merely to pacify a lint checker". FWIW I think the issue here is that the warning was confused by the "case '#':" on the same line as the "while"); I can suppress the warning (in ed-1.13) via the following addition of a comment: --- main_loop.c.orig2016-05-04 14:10:05.263880101 -0400 +++ main_loop.c 2016-05-04 14:11:55.741347000 -0400 @@ -622,7 +622,8 @@ static int exec_command( const char ** c !display_lines( second_addr, second_addr, 0 ) ) return ERR; break; -case '#': while( *(*ibufpp)++ != '\n' ) ; +case '#': /* Skip comment. */ + while( *(*ibufpp)++ != '\n' ) ; break; default : set_error_msg( "Unknown command" ); return ERR; } though, as I said, it's a bug. > I also think that -Wmisleading-indentation should not be enabled by > -Wall nor -Wextra because: >- -Wall is defined in the gcc manual as enabling all the warnings > about constructions that are easy to avoid (or modify to prevent > the > warning), even in conjunction with macros. This is not true of > -Wmisleading-indentation. It's trivial to work around, by changing the indentation. What we have here is a bug. Admittedly it took me a few tries to work out an indentation that the warning was happy with, but I hope to fix that as part of the above bug fix. >- It can't be portably disabled; older versions of gcc do not > accept > '-Wno-misleading-indentation'. (At least 4.1.2 does not accept > it). FWIW "-Wall -Wno-misleading-indentation" works for me with gcc 4.8.3 >- If there are no macros involved, it is independent of code > generation; once tested in the developer's machine there is no > need > to test it again on every user's machine. I don't understand this objection. >- It makes compilation a 0.5-1% slower for every user for no > reason. Where did these numbers come from? >- -Wempty-body is much simpler to test fo
Re: Please, take '-Wmisleading-indentation' out of -Wall
On 04/05/16 19:20, David Malcolm wrote: On Wed, 2016-05-04@18:15 +0200, Antonio Diaz Diaz wrote: - It can't be portably disabled; older versions of gcc do not accept '-Wno-misleading-indentation'. (At least 4.1.2 does not accept it). FWIW "-Wall -Wno-misleading-indentation" works for me with gcc 4.8.3 It should work since GCC 4.4 (7 years old). See https://gcc.gnu.org/wiki/FAQ#wnowarning - -Wempty-body is much simpler to test for, and in general less questionable than -Wmisleading-indentation, yet it is not enabled by -Wall. Many useful warnings are outside -Wall/-Wextra because there were bugs when initially implemented, users complained, they were moved out and then either the bugs were forgotten or they got fixed but nobody bothered to move them again within -Wall/-Wextra. See https://gcc.gnu.org/PR52961 Nowadays it is extremely easy to disable a warning that annoys you (the name is written in the message and you can use #pragmas very selectively), but still quite hard to discover which warning you need to enable that could have found a certain bug. It is also much harder to find regressions in warnings not enabled by -Wall -Wextra because they are not tested as much. I really commend David for being brave and putting the warning in -Wall. The easy way out would have been to say: "I know how to enable it, so let's hide it so that users do not complain to me about bugs that I don't suffer". We (myself included) have done this plenty of times in the past and the result is always the same: bugs don't get fixed, regressions appear, and users complain about missing warnings that are actually already implemented. I think that keeping separated categories of warnings (instead of warning about everything by default) is a valuable feature. Maybe both -Wempty-body and -Wmisleading-indentation (and any future similar options) could be put in a new category (-Wcoding-style or -Wstatic-analysis, for example). I agree that GCC warnings could be better categorized, but your proposed categories are not clearly defined. A better classification would look at the possibilities of false positives, how easy is to silence it, whether it is "undefined/unspecified at runtime", "undefined/unspecif under some conditions but not others", "atypical code that is often/sometimes a bug", "typical code that is often/sometimes a bug", etc. Of course, this would be a lot of work that no one wants to do ;-) Cheers, Manuel.
Re: Please, take '-Wmisleading-indentation' out of -Wall
On May 4, 2016 2:35:38 PM CDT, "Manuel López-Ibáñez" wrote: >On 04/05/16 19:20, David Malcolm wrote: >> On Wed, 2016-05-04@18:15 +0200, Antonio Diaz Diaz wrote: >>> - It can't be portably disabled; older versions of gcc do not >>> accept >>> '-Wno-misleading-indentation'. (At least 4.1.2 does not accept >>> it). >> >> FWIW "-Wall -Wno-misleading-indentation" works for me with gcc 4.8.3 > >It should work since GCC 4.4 (7 years old). See >https://gcc.gnu.org/wiki/FAQ#wnowarning > >>> - -Wempty-body is much simpler to test for, and in general less >>> questionable than -Wmisleading-indentation, yet it is not >>> enabled by >>> -Wall. >>> > >Many useful warnings are outside -Wall/-Wextra because there were bugs >when >initially implemented, users complained, they were moved out and then >either >the bugs were forgotten or they got fixed but nobody bothered to move >them >again within -Wall/-Wextra. See https://gcc.gnu.org/PR52961 > >Nowadays it is extremely easy to disable a warning that annoys you (the >name is >written in the message and you can use #pragmas very selectively), but >still >quite hard to discover which warning you need to enable that could have >found a >certain bug. It is also much harder to find regressions in warnings not >enabled >by -Wall -Wextra because they are not tested as much. > >I really commend David for being brave and putting the warning in >-Wall. The >easy way out would have been to say: "I know how to enable it, so let's >hide it >so that users do not complain to me about bugs that I don't suffer". We >(myself >included) have done this plenty of times in the past and the result is >always >the same: bugs don't get fixed, regressions appear, and users complain >about >missing warnings that are actually already implemented. I personally was dreading seeing how many warnings we would see in RTEMS when this first option first appeared. But we only saw a few warnings and a false positive. One of the warnings was definitely code that was unclear what the author's intent was. Others were worth addressing. The false positive was reported and fixed promptly. David has done well on this one. I am a strong believer that code is written once, read many times, and debugged by unfortunate souls. It needs to be clear and unambiguous. This warning is helpful for that. --Joel RTEMS >>> I think that keeping separated categories of warnings (instead of >>> warning about everything by default) is a valuable feature. Maybe >>> both >>> -Wempty-body and -Wmisleading-indentation (and any future similar >>> options) could be put in a new category (-Wcoding-style or >>> -Wstatic-analysis, for example). > >I agree that GCC warnings could be better categorized, but your >proposed >categories are not clearly defined. A better classification would look >at the >possibilities of false positives, how easy is to silence it, whether it >is >"undefined/unspecified at runtime", "undefined/unspecif under some >conditions >but not others", "atypical code that is often/sometimes a bug", >"typical code >that is often/sometimes a bug", etc. Of course, this would be a lot of >work >that no one wants to do ;-) > >Cheers, > > Manuel. --joel
Re: Please, take '-Wmisleading-indentation' out of -Wall
On Wed, May 4, 2016 at 3:35 PM, Manuel López-Ibáñez wrote: > On 04/05/16 19:20, David Malcolm wrote: >> >> On Wed, 2016-05-04@18:15 +0200, Antonio Diaz Diaz wrote: >>> >>> - It can't be portably disabled; older versions of gcc do not >>> accept >>> '-Wno-misleading-indentation'. (At least 4.1.2 does not accept >>> it). >> >> >> FWIW "-Wall -Wno-misleading-indentation" works for me with gcc 4.8.3 > > > It should work since GCC 4.4 (7 years old). See > https://gcc.gnu.org/wiki/FAQ#wnowarning 4.1.2 is the system compiler for RHEL5, which is most likely why he needs it (EL5 is still actively maintained)
gcc-4.9-20160504 is now available
Snapshot gcc-4.9-20160504 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/4.9-20160504/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 4.9 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-4_9-branch revision 235912 You'll find: gcc-4.9-20160504.tar.bz2 Complete GCC MD5=85b384c7cdbe8221d42d637b2a67b3df SHA1=bf0e33c5c9c7c7c22f18046d2193bcc9952291bc Diffs from 4.9-20160427 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-4.9 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.