On Thu, 2020-04-23 at 16:05 -0500, Qing Zhao wrote:
> Hi, Richard,
>
> This is the 3rd version of the patch, updated based on your previous
> comments.
>
> Please take a look at it and let me know whether it’s okay to commit?
>
> Thanks a lot for all your help.
>
> Qing.
>
> gcc/ChangeLog:
>
> 2020-04-22 qing zhao <qing.z...@oracle.com>
>
> PR c/94230
> * common.opt: Add -flarge-source-files.
> * doc/invoke.texi: Document it.
> * toplev.c (process_options): set line_table-
> >default_range_bits
> to 0 when flag_large_source_files is true.
>
>
> gcc/c-family/ChangeLog:
>
> 2020-04-22 qing zhao <qing.z...@oracle.com>
>
> PR c/94230
> * c-indentation.c (get_visual_column): Add a hint to use the
> new
> -flarge-source-files option.
>
> gcc/testsuite/ChangeLog:
>
> 2020-04-22 qing zhao <qing.z...@oracle.com>
>
> PR c/94230
> * gcc.dg/plugin/location-overflow-test-1.c (fn_1): New message
> to
> provide hint to use the new -flarge-source-files option.
>
>
> > On Apr 23, 2020, at 1:27 PM, Richard Sandiford <
> > richard.sandif...@arm.com> wrote:
> >
> > Qing Zhao <qing.z...@oracle.com> writes:
> > > ---
> > > gcc/c-family/c-indentation.c | 3 +++
> > > gcc/common.opt | 5 +++++
> > > gcc/doc/invoke.texi | 15
> > > ++++++++++++++-
> > > gcc/testsuite/gcc.dg/plugin/location-overflow-test-1.c | 2 +-
> > > gcc/toplev.c | 3 +++
> > > 5 files changed, 26 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-
> > > indentation.c
> > > index f737555..7074b10 100644
> > > --- a/gcc/c-family/c-indentation.c
> > > +++ b/gcc/c-family/c-indentation.c
> > > @@ -67,6 +67,9 @@ get_visual_column (expanded_location exploc,
> > > location_t loc,
> > > "%<-Wmisleading-indentation%> is disabled from this
> > > point"
> > > " onwards, since column-tracking was disabled due to"
> > > " the size of the code/headers");
> > > + inform (loc,
> > > + "please add %<-flarge-source-files%> to invoke more"
> > > + " column-tracking for large source files");
> > > }
> > > return false;
> > > }
> >
> > This should be conditional on !flag_large_source_files.
> >
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index 4368910..10a3d5b 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -1597,6 +1597,11 @@ fkeep-gc-roots-live
> > > Common Undocumented Report Var(flag_keep_gc_roots_live)
> > > Optimization
> > > ; Always keep a pointer to a live memory block
> > >
> > > +flarge-source-files
> > > +Common Report Var(flag_large_source_files) Init(0)
> > > +Adjust GCC to cope with large source files to provide more
> > > accurate
> > > +column information.
> > > +
> >
> > I'm having difficulty suggesting wording here, but I think would be
> > good
> > to mention the downside. How about:
> >
> > ----------------------
> > Improve GCC's ability to track column numbers in large source
> > files,
> > at the expense of slower compilation.
> > ----------------------
> >
> > > floop-parallelize-all
> > > Common Report Var(flag_loop_parallelize_all) Optimization
> > > Mark all loops as parallel.
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index 96a9516..c6ea9ef 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -574,7 +574,7 @@ Objective-C and Objective-C++ Dialects}.
> > > -fdebug-cpp -fdirectives-only -fdollars-in-identifiers @gol
> > > -fexec-charset=@var{charset} -fextended-identifiers @gol
> > > -finput-charset=@var{charset} -fmacro-prefix-map=@var{old}=@var{
> > > new} @gol
> > > --fmax-include-depth=@var{depth} @gol
> > > +-fmax-include-depth=@var{depth} -flarge-source-files @gol
> > > -fno-canonical-system-headers -fpch-deps -fpch-preprocess @gol
> > > -fpreprocessed -ftabstop=@var{width} -ftrack-macro-
> > > expansion @gol
> > > -fwide-exec-charset=@var{charset} -fworking-directory @gol
> >
> > This should be kept in alphabetical order, so after -finput-
> > charset.
> >
> > > @@ -14151,6 +14151,19 @@ This option may be useful in conjunction
> > > with the @option{-B} or
> > > perform additional processing of the program source between
> > > normal preprocessing and compilation.
> > >
> > > +@item -flarge-source-files
> > > +@opindex flarge-source-files
> > > +Adjust GCC to cope with large source files to provide more
> > > accurate
> > > +column information.
> > > +By default, GCC will lose accurate column information if the
> > > source
> > > +file is very large.
> > > +If this option is provided, GCC will adapt accordingly to
> > > provide more
> > > +accurate column information.
> > > +This option may be useful when any user hits the @option{-
> > > Wmisleading-indent}
> > > +note, "is disabled from this point onwards, since column-
> > > tracking was disabled
> > > +due to the size of the code/headers", or hits the limit at which
> > > columns get
> > > +dropped from diagnostics.
> > > +
> >
> > On a similar vein, how about:
> >
> > ----------------------
> > Adjust GCC to expect large source files, at the expense of slower
> > compilation and higher memory usage.
> >
> > Specifically, GCC normally tracks both column numbers and line
> > numbers
> > within source files and it normally prints both of these numbers in
> > diagnostics. However, once it has processed a certain number of
> > source
> > lines, it stops tracking column numbers and only tracks line
> > numbers.
> > This means that diagnostics for later lines do not include column
> > numbers.
> > It also means that options like @option{-Wmisleading-indent} cease
> > to work
> > at that point, although the compiler prints a note if this happens.
> > Passing @option{-flarge-source-files} significantly increases the
> > number
> > of source lines that GCC can process before it stops tracking
> > column
> > numbers.
> > ----------------------
> >
> > Thanks,
> > Richard
> Hi, Richard,
>
> This is the 3rd version of the patch, updated based on your previous comments.
Thanks for working on this, and to Richard for the useful comments.
> Please take a look at it and let me know whether it’s okay to commit?
I like the overall approach.
Some nits inline.
> gcc/ChangeLog:
>
> 2020-04-22 qing zhao <qing.z...@oracle.com>
>
> PR c/94230
> * common.opt: Add -flarge-source-files.
> * doc/invoke.texi: Document it.
> * toplev.c (process_options): set line_table->default_range_bits
> to 0 when flag_large_source_files is true.
>
>
> gcc/c-family/ChangeLog:
>
> 2020-04-22 qing zhao <qing.z...@oracle.com>
>
> PR c/94230
> * c-indentation.c (get_visual_column): Add a hint to use the new
> -flarge-source-files option.
>
> gcc/testsuite/ChangeLog:
>
> 2020-04-22 qing zhao <qing.z...@oracle.com>
>
> PR c/94230
> * gcc.dg/plugin/location-overflow-test-1.c (fn_1): New message to
> provide hint to use the new -flarge-source-files option.
Maybe: "Verify that hint about -flarge-source-files is emitted".
> From 97696d2c1c146fd7d3161f592d42cd7d73c6d5a8 Mon Sep 17 00:00:00 2001
> From: qing zhao <qing.z...@oracle.com>
> Date: Thu, 23 Apr 2020 16:55:24 -0400
> Subject: [PATCH] Fix PR94230
>
> ---
> gcc/c-family/c-indentation.c | 4 ++++
> gcc/common.opt | 5 +++++
> gcc/doc/invoke.texi | 19
> +++++++++++++++++--
> .../gcc.dg/plugin/location-overflow-test-1.c | 2 +-
> gcc/toplev.c | 3 +++
> 5 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
> index f737555..e929030 100644
> --- a/gcc/c-family/c-indentation.c
> +++ b/gcc/c-family/c-indentation.c
> @@ -67,6 +67,10 @@ get_visual_column (expanded_location exploc, location_t
> loc,
> "%<-Wmisleading-indentation%> is disabled from this point"
> " onwards, since column-tracking was disabled due to"
> " the size of the code/headers");
> + if (!flag_large_source_files)
> + inform (loc,
> + "please add %<-flarge-source-files%> to invoke more"
> + " column-tracking for large source files");
It's great having a hint here, but I don't love the wording of the
hint.
Maybe reword to:
"adding %<-flarge-source-files%> will allow for more column-tracking
support, at the expense of compilation time and memory".
or somesuch.
> }
> return false;
> }
[...]
> @@ -14151,6 +14151,21 @@ This option may be useful in conjunction with the
> @option{-B} or
> perform additional processing of the program source between
> normal preprocessing and compilation.
>
> +@item -flarge-source-files
> +@opindex flarge-source-files
> +Adjust GCC to expect large source files, at the expense of slower
> +compilation and higher memory usage.
> +
> +Specifically, GCC normally tracks both column numbers and line numbers
> +within source files and it normally prints both of these numbers in
> +diagnostics. However, once it has processed a certain number of source
> +lines, it stops tracking column numbers and only tracks line numbers.
> +This means that diagnostics for later lines do not include column numbers.
> +It also means that options like @option{-Wmisleading-indent} cease to work
^^^^^^^^^^^^^^^^^^^
This should be: -Wmisleading-indentation
> +at that point, although the compiler prints a note if this happens.
> +Passing @option{-flarge-source-files} significantly increases the number
> +of source lines that GCC can process before it stops tracking column.
^^^^^^
This should be: columns
> @node Assembler Options
> diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-1.c
> b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-1.c
> index 1a80a66..fbc8211 100644
> --- a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-1.c
> +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-1.c
> @@ -13,7 +13,7 @@ int
> fn_1 (int flag)
> {
> int x = 4, y = 5;
> - if (flag) x = 3; y = 2; /* { dg-message "-:disabled from this point" } */
> + if (flag) x = 3; y = 2; /* { dg-message "-:disabled from this point" "add
> '-flarge-source-files'" } */
May need updating to match the hint message.
[...]
This is OK for stage 1 with those nits fixed.
There was talk earlier in the thread about fixing this in GCC 10.
Richi/Jakub: is this OK for stage 4? Although it adds a new command-
line option, it's a workaround for a regression introduced in GCC 6 and
should be low risk.
Thanks
Dave
