Message ID | 5672A5C0.2060106@mentor.com |
---|---|
State | New |
Headers | show |
On 17/12/15 13:08, Tom de Vries wrote: > Hi, > > Consider this patch, which reduces max_len of the oacc function > attribute to 0: > ... > diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c > index 8556b70..60f4ad3 100644 > --- a/gcc/fortran/f95-lang.c > +++ b/gcc/fortran/f95-lang.c > @@ -93,7 +93,7 @@ static const struct attribute_spec > gfc_attribute_table[] = > affects_type_identity } */ > { "omp declare target", 0, 0, true, false, false, > gfc_handle_omp_declare_target_attribute, false }, > - { "oacc function", 0, -1, true, false, false, > + { "oacc function", 0, 0, true, false, false, > gfc_handle_omp_declare_target_attribute, false }, > { NULL, 0, 0, false, false, false, NULL, false } > }; > ... > > The patch is obviously incorrect, but the idea here is to try to trigger > this error in decl_attributes: > ... > else if (list_length (args) < spec->min_length > || (spec->max_length >= 0 > && list_length (args) > spec->max_length)) > { > error ("wrong number of arguments specified for %qE" > " attribute", > name); > ... > > When running goacc.exp=routine-4.f90, we trigger the error, but then run > into an assert. > > The assert is caused by the fact that %qE is not handled by the fortran > format decoder gfc_format_decoder, so this assert triggers in pp_format: > ... > ok = pp_format_decoder (pp) (pp, text, p, > precision, wide, plus, hash); > gcc_assert (ok); > ... > > > So, it seems that we call decl_attributes from the fortran frontend > without installing a format decoder that can handle any potential errors. > > This patch attempts to fix that, but having little experience in both > diagnostics and fortran frontend, I'm not sure if this is the right way. > > After applying the patch, the assert is fixed and we can see the actual > error without having to start up the debugger: > ... > src/gcc/testsuite/gfortran.dg/goacc/routine-4.f90:121:0: Error: wrong > number of arguments specified for ‘oacc function’ attribute > ... > > Thanks, > - Tom > > 0001-Use-gfc_decl_attributes-in-fortran-frontend.patch > > > Use gfc_decl_attributes in fortran frontend > > --- > gcc/fortran/error.c | 18 ++++++++++++++++-- > gcc/fortran/gfortran.h | 2 ++ > gcc/fortran/trans-decl.c | 18 ++++++++++++++---- > 3 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/gcc/fortran/error.c b/gcc/fortran/error.c > index 8f57aff..fd66d75 100644 > --- a/gcc/fortran/error.c > +++ b/gcc/fortran/error.c > @@ -1417,11 +1417,18 @@ gfc_errors_to_warnings (bool f) > } > > void > -gfc_diagnostics_init (void) > +gfc_diagnostics_fortran (void) > { > diagnostic_starter (global_dc) = gfc_diagnostic_starter; > diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer; > diagnostic_format_decoder (global_dc) = gfc_format_decoder; > +} > + > +void > +gfc_diagnostics_init (void) > +{ > + gfc_diagnostics_fortran (); > + > global_dc->caret_chars[0] = '1'; > global_dc->caret_chars[1] = '2'; > pp_warning_buffer = new (XNEW (output_buffer)) output_buffer (); > @@ -1433,13 +1440,20 @@ gfc_diagnostics_init (void) > } > > void > -gfc_diagnostics_finish (void) > +gfc_diagnostics_tree (void) > { > tree_diagnostics_defaults (global_dc); > /* We still want to use the gfc starter and finalizer, not the tree > defaults. */ > diagnostic_starter (global_dc) = gfc_diagnostic_starter; > diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer; > +} > + > +void > +gfc_diagnostics_finish (void) > +{ > + gfc_diagnostics_tree (); > + > global_dc->caret_chars[0] = '^'; > global_dc->caret_chars[1] = '^'; > } > diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h > index d203c32..1f7cdc2 100644 > --- a/gcc/fortran/gfortran.h > +++ b/gcc/fortran/gfortran.h > @@ -2687,6 +2687,8 @@ bool gfc_find_sym_in_expr (gfc_symbol *, gfc_expr *); > void gfc_error_init_1 (void); > void gfc_diagnostics_init (void); > void gfc_diagnostics_finish (void); > +void gfc_diagnostics_fortran (void); > +void gfc_diagnostics_tree (void); > void gfc_buffer_error (bool); > > const char *gfc_print_wide_char (gfc_char_t); > diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c > index 8c4fa03..9ed1d07 100644 > --- a/gcc/fortran/trans-decl.c > +++ b/gcc/fortran/trans-decl.c > @@ -1326,6 +1326,16 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list) > } > > > +static tree > +gfc_decl_attributes (tree *node, tree attributes, int flags) > +{ > + tree res; > + gfc_diagnostics_tree (); > + res = decl_attributes (node, attributes, flags); > + gfc_diagnostics_fortran (); > + return res; > +} > + > static void build_function_decl (gfc_symbol * sym, bool global); > > > @@ -1567,7 +1577,7 @@ gfc_get_symbol_decl (gfc_symbol * sym) > > /* Add attributes to variables. Functions are handled elsewhere. */ > attributes = add_attributes_to_decl (sym->attr, NULL_TREE); > - decl_attributes (&decl, attributes, 0); > + gfc_decl_attributes (&decl, attributes, 0); > > /* Symbols from modules should have their assembler names mangled. > This is done here rather than in gfc_finish_var_decl because it > @@ -1802,7 +1812,7 @@ get_proc_pointer_decl (gfc_symbol *sym) > set_decl_tls_model (decl, decl_default_tls_model (decl)); > > attributes = add_attributes_to_decl (sym->attr, NULL_TREE); > - decl_attributes (&decl, attributes, 0); > + gfc_decl_attributes (&decl, attributes, 0); > > return decl; > } > @@ -1995,7 +2005,7 @@ module_sym: > TREE_PUBLIC (fndecl) = 1; > > attributes = add_attributes_to_decl (sym->attr, NULL_TREE); > - decl_attributes (&fndecl, attributes, 0); > + gfc_decl_attributes (&fndecl, attributes, 0); > > gfc_set_decl_assembler_name (fndecl, mangled_name); > > @@ -2097,7 +2107,7 @@ build_function_decl (gfc_symbol * sym, bool global) > TREE_USED (fndecl) = 1; > > attributes = add_attributes_to_decl (attr, NULL_TREE); > - decl_attributes (&fndecl, attributes, 0); > + gfc_decl_attributes (&fndecl, attributes, 0); > > /* Figure out the return type of the declared function, and build a > RESULT_DECL for it. If this is a subroutine with alternate >
On 01/13/2016 05:21 PM, Tom de Vries wrote: > On 17/12/15 13:08, Tom de Vries wrote: >> Hi, >> >> Consider this patch, which reduces max_len of the oacc function >> attribute to 0: >> ... >> diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c >> index 8556b70..60f4ad3 100644 >> --- a/gcc/fortran/f95-lang.c >> +++ b/gcc/fortran/f95-lang.c >> @@ -93,7 +93,7 @@ static const struct attribute_spec >> gfc_attribute_table[] = >> affects_type_identity } */ >> { "omp declare target", 0, 0, true, false, false, >> gfc_handle_omp_declare_target_attribute, false }, >> - { "oacc function", 0, -1, true, false, false, >> + { "oacc function", 0, 0, true, false, false, >> gfc_handle_omp_declare_target_attribute, false }, >> { NULL, 0, 0, false, false, false, NULL, false } >> }; >> ... >> >> The patch is obviously incorrect, but the idea here is to try to trigger >> this error in decl_attributes: >> ... >> else if (list_length (args) < spec->min_length >> || (spec->max_length >= 0 >> && list_length (args) > spec->max_length)) >> { >> error ("wrong number of arguments specified for %qE" >> " attribute", >> name); >> ... >> >> When running goacc.exp=routine-4.f90, we trigger the error, but then run >> into an assert. >> >> The assert is caused by the fact that %qE is not handled by the fortran >> format decoder gfc_format_decoder, so this assert triggers in pp_format: >> ... >> ok = pp_format_decoder (pp) (pp, text, p, >> precision, wide, plus, hash); >> gcc_assert (ok); >> ... >> >> >> So, it seems that we call decl_attributes from the fortran frontend >> without installing a format decoder that can handle any potential errors. >> >> This patch attempts to fix that, but having little experience in both >> diagnostics and fortran frontend, I'm not sure if this is the right way. >> >> After applying the patch, the assert is fixed and we can see the actual >> error without having to start up the debugger: >> ... >> src/gcc/testsuite/gfortran.dg/goacc/routine-4.f90:121:0: Error: wrong >> number of arguments specified for ‘oacc function’ attribute >> ... >> >> Thanks, >> - Tom >> >> 0001-Use-gfc_decl_attributes-in-fortran-frontend.patch >> >> >> Use gfc_decl_attributes in fortran frontend >> >> --- >> gcc/fortran/error.c | 18 ++++++++++++++++-- >> gcc/fortran/gfortran.h | 2 ++ >> gcc/fortran/trans-decl.c | 18 ++++++++++++++---- >> 3 files changed, 32 insertions(+), 6 deletions(-) >> >> diff --git a/gcc/fortran/error.c b/gcc/fortran/error.c >> index 8f57aff..fd66d75 100644 >> --- a/gcc/fortran/error.c >> +++ b/gcc/fortran/error.c >> @@ -1417,11 +1417,18 @@ gfc_errors_to_warnings (bool f) >> } >> >> void >> -gfc_diagnostics_init (void) >> +gfc_diagnostics_fortran (void) >> { >> diagnostic_starter (global_dc) = gfc_diagnostic_starter; >> diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer; >> diagnostic_format_decoder (global_dc) = gfc_format_decoder; >> +} >> + >> +void >> +gfc_diagnostics_init (void) >> +{ >> + gfc_diagnostics_fortran (); >> + >> global_dc->caret_chars[0] = '1'; >> global_dc->caret_chars[1] = '2'; >> pp_warning_buffer = new (XNEW (output_buffer)) output_buffer (); >> @@ -1433,13 +1440,20 @@ gfc_diagnostics_init (void) >> } >> >> void >> -gfc_diagnostics_finish (void) >> +gfc_diagnostics_tree (void) >> { >> tree_diagnostics_defaults (global_dc); >> /* We still want to use the gfc starter and finalizer, not the tree >> defaults. */ >> diagnostic_starter (global_dc) = gfc_diagnostic_starter; >> diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer; >> +} >> + >> +void >> +gfc_diagnostics_finish (void) >> +{ >> + gfc_diagnostics_tree (); >> + >> global_dc->caret_chars[0] = '^'; >> global_dc->caret_chars[1] = '^'; >> } >> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h >> index d203c32..1f7cdc2 100644 >> --- a/gcc/fortran/gfortran.h >> +++ b/gcc/fortran/gfortran.h >> @@ -2687,6 +2687,8 @@ bool gfc_find_sym_in_expr (gfc_symbol *, >> gfc_expr *); >> void gfc_error_init_1 (void); >> void gfc_diagnostics_init (void); >> void gfc_diagnostics_finish (void); >> +void gfc_diagnostics_fortran (void); >> +void gfc_diagnostics_tree (void); >> void gfc_buffer_error (bool); >> >> const char *gfc_print_wide_char (gfc_char_t); >> diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c >> index 8c4fa03..9ed1d07 100644 >> --- a/gcc/fortran/trans-decl.c >> +++ b/gcc/fortran/trans-decl.c >> @@ -1326,6 +1326,16 @@ add_attributes_to_decl (symbol_attribute >> sym_attr, tree list) >> } >> >> >> +static tree >> +gfc_decl_attributes (tree *node, tree attributes, int flags) >> +{ >> + tree res; >> + gfc_diagnostics_tree (); >> + res = decl_attributes (node, attributes, flags); >> + gfc_diagnostics_fortran (); >> + return res; >> +} >> + >> static void build_function_decl (gfc_symbol * sym, bool global); >> >> >> @@ -1567,7 +1577,7 @@ gfc_get_symbol_decl (gfc_symbol * sym) >> >> /* Add attributes to variables. Functions are handled elsewhere. */ >> attributes = add_attributes_to_decl (sym->attr, NULL_TREE); >> - decl_attributes (&decl, attributes, 0); >> + gfc_decl_attributes (&decl, attributes, 0); >> >> /* Symbols from modules should have their assembler names mangled. >> This is done here rather than in gfc_finish_var_decl because it >> @@ -1802,7 +1812,7 @@ get_proc_pointer_decl (gfc_symbol *sym) >> set_decl_tls_model (decl, decl_default_tls_model (decl)); >> >> attributes = add_attributes_to_decl (sym->attr, NULL_TREE); >> - decl_attributes (&decl, attributes, 0); >> + gfc_decl_attributes (&decl, attributes, 0); >> >> return decl; >> } >> @@ -1995,7 +2005,7 @@ module_sym: >> TREE_PUBLIC (fndecl) = 1; >> >> attributes = add_attributes_to_decl (sym->attr, NULL_TREE); >> - decl_attributes (&fndecl, attributes, 0); >> + gfc_decl_attributes (&fndecl, attributes, 0); >> >> gfc_set_decl_assembler_name (fndecl, mangled_name); >> >> @@ -2097,7 +2107,7 @@ build_function_decl (gfc_symbol * sym, bool global) >> TREE_USED (fndecl) = 1; >> >> attributes = add_attributes_to_decl (attr, NULL_TREE); >> - decl_attributes (&fndecl, attributes, 0); >> + gfc_decl_attributes (&fndecl, attributes, 0); >> >> /* Figure out the return type of the declared function, and build a >> RESULT_DECL for it. If this is a subroutine with alternate >> >
Use gfc_decl_attributes in fortran frontend --- gcc/fortran/error.c | 18 ++++++++++++++++-- gcc/fortran/gfortran.h | 2 ++ gcc/fortran/trans-decl.c | 18 ++++++++++++++---- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/gcc/fortran/error.c b/gcc/fortran/error.c index 8f57aff..fd66d75 100644 --- a/gcc/fortran/error.c +++ b/gcc/fortran/error.c @@ -1417,11 +1417,18 @@ gfc_errors_to_warnings (bool f) } void -gfc_diagnostics_init (void) +gfc_diagnostics_fortran (void) { diagnostic_starter (global_dc) = gfc_diagnostic_starter; diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer; diagnostic_format_decoder (global_dc) = gfc_format_decoder; +} + +void +gfc_diagnostics_init (void) +{ + gfc_diagnostics_fortran (); + global_dc->caret_chars[0] = '1'; global_dc->caret_chars[1] = '2'; pp_warning_buffer = new (XNEW (output_buffer)) output_buffer (); @@ -1433,13 +1440,20 @@ gfc_diagnostics_init (void) } void -gfc_diagnostics_finish (void) +gfc_diagnostics_tree (void) { tree_diagnostics_defaults (global_dc); /* We still want to use the gfc starter and finalizer, not the tree defaults. */ diagnostic_starter (global_dc) = gfc_diagnostic_starter; diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer; +} + +void +gfc_diagnostics_finish (void) +{ + gfc_diagnostics_tree (); + global_dc->caret_chars[0] = '^'; global_dc->caret_chars[1] = '^'; } diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index d203c32..1f7cdc2 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2687,6 +2687,8 @@ bool gfc_find_sym_in_expr (gfc_symbol *, gfc_expr *); void gfc_error_init_1 (void); void gfc_diagnostics_init (void); void gfc_diagnostics_finish (void); +void gfc_diagnostics_fortran (void); +void gfc_diagnostics_tree (void); void gfc_buffer_error (bool); const char *gfc_print_wide_char (gfc_char_t); diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index 8c4fa03..9ed1d07 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -1326,6 +1326,16 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list) } +static tree +gfc_decl_attributes (tree *node, tree attributes, int flags) +{ + tree res; + gfc_diagnostics_tree (); + res = decl_attributes (node, attributes, flags); + gfc_diagnostics_fortran (); + return res; +} + static void build_function_decl (gfc_symbol * sym, bool global); @@ -1567,7 +1577,7 @@ gfc_get_symbol_decl (gfc_symbol * sym) /* Add attributes to variables. Functions are handled elsewhere. */ attributes = add_attributes_to_decl (sym->attr, NULL_TREE); - decl_attributes (&decl, attributes, 0); + gfc_decl_attributes (&decl, attributes, 0); /* Symbols from modules should have their assembler names mangled. This is done here rather than in gfc_finish_var_decl because it @@ -1802,7 +1812,7 @@ get_proc_pointer_decl (gfc_symbol *sym) set_decl_tls_model (decl, decl_default_tls_model (decl)); attributes = add_attributes_to_decl (sym->attr, NULL_TREE); - decl_attributes (&decl, attributes, 0); + gfc_decl_attributes (&decl, attributes, 0); return decl; } @@ -1995,7 +2005,7 @@ module_sym: TREE_PUBLIC (fndecl) = 1; attributes = add_attributes_to_decl (sym->attr, NULL_TREE); - decl_attributes (&fndecl, attributes, 0); + gfc_decl_attributes (&fndecl, attributes, 0); gfc_set_decl_assembler_name (fndecl, mangled_name); @@ -2097,7 +2107,7 @@ build_function_decl (gfc_symbol * sym, bool global) TREE_USED (fndecl) = 1; attributes = add_attributes_to_decl (attr, NULL_TREE); - decl_attributes (&fndecl, attributes, 0); + gfc_decl_attributes (&fndecl, attributes, 0); /* Figure out the return type of the declared function, and build a RESULT_DECL for it. If this is a subroutine with alternate