public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
@ 2020-04-03 19:55 Qing Zhao
  2020-04-08 19:39 ` PING " Qing Zhao
  0 siblings, 1 reply; 16+ messages in thread
From: Qing Zhao @ 2020-04-03 19:55 UTC (permalink / raw)
  To: dmalcolm, jakub; +Cc: gcc-patches

Hi, David and Jakub,

Per the discussion we had for PR94230: provide an option to change the size limitation for -Wmisleading-indent
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94230 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94230>

I come up with the following simple patch per David’s suggestion:

Provide a new first class option -flocation-ranges to control enabling/disablng range tracking when recording source locations.
The default value for this option is enabling the range tracking.

When specify -fno-location-ranges, disable the range tracking to save space for column tracking. 

I have tested this GCC to build our huge application by adding -fno-location-ranges, the whole build completed without any issue. and
-Wmisleading-indent also emitted several helpful warning message during building.

I am running bootstrap right now.

Could you please take a look at the attached patch?

thanks.

Qing





^ permalink raw reply	[flat|nested] 16+ messages in thread

* PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-03 19:55 [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent Qing Zhao
@ 2020-04-08 19:39 ` Qing Zhao
  2020-04-15 20:30   ` Fwd: " Qing Zhao
  2020-04-21 14:04   ` Richard Sandiford
  0 siblings, 2 replies; 16+ messages in thread
From: Qing Zhao @ 2020-04-08 19:39 UTC (permalink / raw)
  To: dmalcolm, jakub; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

Hi,

Please take a look at the attached patch and let me know your comments.

Thanks.

Qing

gcc/ChangeLog:

2020-04-03  qing zhao  <qing.zhao@oracle.com>

	* common.opt: Add -flocation-ranges.
	* doc/invoke.texi: Document it.
	* toplev.c (process_options): set line_table->default_range_bits
	to 0 when flag_location_ranges is false. 


[-- Attachment #2: PR94230.patch --]
[-- Type: application/octet-stream, Size: 2183 bytes --]

diff --git a/gcc/common.opt b/gcc/common.opt
index 4368910..041cfd7 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1597,6 +1597,10 @@ fkeep-gc-roots-live
 Common Undocumented Report Var(flag_keep_gc_roots_live) Optimization
 ; Always keep a pointer to a live memory block
 
+flocation-ranges
+Common Report Var(flag_location_ranges) Init(1)
+Enable range tracking when recording source locations.
+
 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..2faac62 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} -flocation-ranges @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
@@ -14151,6 +14151,13 @@ 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 -flocation-ranges
+@opindex flocation-ranges
+Enable range tracking when recording source locations.
+By default, GCC enables range tracking when recording source locations.
+If disable range tracking by -fno-location-ranges, more location space
+will be saved for column tracking.
+
 @end table
 
 @node Assembler Options
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 4c8be50..5d035de 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1854,6 +1854,9 @@ process_options (void)
     hash_table_sanitize_eq_limit
       = param_hash_table_verification_limit;
 
+  if (!flag_location_ranges)
+    line_table->default_range_bits = 0;
+
   /* Please don't change global_options after this point, those changes won't
      be reflected in optimization_{default,current}_node.  */
 }

[-- Attachment #3: Type: text/plain, Size: 1374 bytes --]



> Begin forwarded message:
> 
> From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org>
> Subject: [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent 
> Date: April 3, 2020 at 2:55:53 PM CDT
> To: dmalcolm@redhat.com, jakub@redhat.com
> Cc: gcc-patches@gcc.gnu.org
> Reply-To: Qing Zhao <QING.ZHAO@ORACLE.COM>
> 
> Hi, David and Jakub,
> 
> Per the discussion we had for PR94230: provide an option to change the size limitation for -Wmisleading-indent
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94230 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94230>
> 
> I come up with the following simple patch per David’s suggestion:
> 
> Provide a new first class option -flocation-ranges to control enabling/disablng range tracking when recording source locations.
> The default value for this option is enabling the range tracking.
> 
> When specify -fno-location-ranges, disable the range tracking to save space for column tracking. 
> 
> I have tested this GCC to build our huge application by adding -fno-location-ranges, the whole build completed without any issue. and
> -Wmisleading-indent also emitted several helpful warning message during building.
> 
> I am running bootstrap right now.
> 
> Could you please take a look at the attached patch?
> 
> thanks.
> 
> Qing
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Fwd: PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-08 19:39 ` PING " Qing Zhao
@ 2020-04-15 20:30   ` Qing Zhao
  2020-04-21 14:04   ` Richard Sandiford
  1 sibling, 0 replies; 16+ messages in thread
From: Qing Zhao @ 2020-04-15 20:30 UTC (permalink / raw)
  To: dmalcolm, jakub; +Cc: gcc-patches

Ping.

We need this patch for our product building.

thanks.

Qing

> Begin forwarded message:
> 
> From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org>
> Subject: PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent 
> Date: April 8, 2020 at 2:39:22 PM CDT
> To: dmalcolm@redhat.com, jakub@redhat.com
> Cc: gcc-patches@gcc.gnu.org
> Reply-To: Qing Zhao <QING.ZHAO@ORACLE.COM>
> 
> Hi,
> 
> Please take a look at the attached patch and let me know your comments.
> 
> Thanks.
> 
> Qing
> 
> gcc/ChangeLog:
> 
> 2020-04-03  qing zhao  <qing.zhao@oracle.com>
> 
> 	* common.opt: Add -flocation-ranges.
> 	* doc/invoke.texi: Document it.
> 	* toplev.c (process_options): set line_table->default_range_bits
> 	to 0 when flag_location_ranges is false. 
> 
> 
> 
>> Begin forwarded message:
>> 
>> From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org>
>> Subject: [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent 
>> Date: April 3, 2020 at 2:55:53 PM CDT
>> To: dmalcolm@redhat.com, jakub@redhat.com
>> Cc: gcc-patches@gcc.gnu.org
>> Reply-To: Qing Zhao <QING.ZHAO@ORACLE.COM>
>> 
>> Hi, David and Jakub,
>> 
>> Per the discussion we had for PR94230: provide an option to change the size limitation for -Wmisleading-indent
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94230 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94230>
>> 
>> I come up with the following simple patch per David’s suggestion:
>> 
>> Provide a new first class option -flocation-ranges to control enabling/disablng range tracking when recording source locations.
>> The default value for this option is enabling the range tracking.
>> 
>> When specify -fno-location-ranges, disable the range tracking to save space for column tracking. 
>> 
>> I have tested this GCC to build our huge application by adding -fno-location-ranges, the whole build completed without any issue. and
>> -Wmisleading-indent also emitted several helpful warning message during building.
>> 
>> I am running bootstrap right now.
>> 
>> Could you please take a look at the attached patch?
>> 
>> thanks.
>> 
>> Qing
>> 
>> 
>> 
>> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-08 19:39 ` PING " Qing Zhao
  2020-04-15 20:30   ` Fwd: " Qing Zhao
@ 2020-04-21 14:04   ` Richard Sandiford
  2020-04-21 15:21     ` David Malcolm
  1 sibling, 1 reply; 16+ messages in thread
From: Richard Sandiford @ 2020-04-21 14:04 UTC (permalink / raw)
  To: Qing Zhao via Gcc-patches; +Cc: dmalcolm, jakub, Qing Zhao

Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hi,
>
> Please take a look at the attached patch and let me know your comments.
>
> Thanks.
>
> Qing

Acting as a reviewer of last resort since this isn't really my area,
but FWIW, I agree losing column tracking at the current limit is a
genuine regression and should be fixed for GCC 10.

> gcc/ChangeLog:
>
> 2020-04-03  qing zhao  <qing.zhao@oracle.com>
>

Please add:

	PR c/94230

> 	* common.opt: Add -flocation-ranges.
> 	* doc/invoke.texi: Document it.
> 	* toplev.c (process_options): set line_table->default_range_bits
> 	to 0 when flag_location_ranges is false. 

I think it would be worth adding a hint to use the new option to
get_visual_column, when warning about column tracking being disabled.
This should probably be a second inform(), immediately after the
current one.

> @@ -14151,6 +14151,13 @@ 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 -flocation-ranges
> +@opindex flocation-ranges

Normally the documented option should be the non-default one,
so -fno-... in this case.

> +Enable range tracking when recording source locations.
> +By default, GCC enables range tracking when recording source locations.
> +If disable range tracking by -fno-location-ranges, more location space
> +will be saved for column tracking.

My understanding is that the patch doesn't actually disable location-range
tracking, but simply uses a less efficient form for *all* ranges, rather
than only using the less efficient form for ranges that aren't "caret at
start, length < 64 chars".

I know you're simply following the suggestion in the PR, sorry,
but I wonder if the option should instead be:

-flarge-source-files

since that seems like a more user-facing concept.  The option would
tell GCC that the source files are likely to be very large and that
GCC should adapt accordingly.  In particular, the option makes GCC
cope with more source lines at the expense of slowing down compilation
and using more memory.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-21 14:04   ` Richard Sandiford
@ 2020-04-21 15:21     ` David Malcolm
  2020-04-21 18:46       ` Richard Sandiford
  0 siblings, 1 reply; 16+ messages in thread
From: David Malcolm @ 2020-04-21 15:21 UTC (permalink / raw)
  To: Richard Sandiford, Qing Zhao via Gcc-patches; +Cc: jakub, Qing Zhao

On Tue, 2020-04-21 at 15:04 +0100, Richard Sandiford wrote:
> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > Hi,
> > 
> > Please take a look at the attached patch and let me know your
> > comments.
> > 
> > Thanks.
> > 
> > Qing
> 
> Acting as a reviewer of last resort since this isn't really my area,

Sorry; life has been crazy lately.

> but FWIW, I agree losing column tracking at the current limit is a
> genuine regression and should be fixed for GCC 10.

Is this a regression in GCC 10 though?  I think this has been the case
since GCC 6 onwards.

> > gcc/ChangeLog:
> > 
> > 2020-04-03  qing zhao  <qing.zhao@oracle.com>
> > 
> 
> Please add:
> 
> 	PR c/94230
> 
> > 	* common.opt: Add -flocation-ranges.
> > 	* doc/invoke.texi: Document it.
> > 	* toplev.c (process_options): set line_table-
> > >default_range_bits
> > 	to 0 when flag_location_ranges is false. 
> 
> I think it would be worth adding a hint to use the new option to
> get_visual_column, when warning about column tracking being disabled.
> This should probably be a second inform(), immediately after the
> current one.
> 
> > @@ -14151,6 +14151,13 @@ 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 -flocation-ranges
> > +@opindex flocation-ranges
> 
> Normally the documented option should be the non-default one,
> so -fno-... in this case.
> 
> > +Enable range tracking when recording source locations.
> > +By default, GCC enables range tracking when recording source
> > locations.
> > +If disable range tracking by -fno-location-ranges, more location
> > space
> > +will be saved for column tracking.
> 
> My understanding is that the patch doesn't actually disable location-
> range
> tracking, but simply uses a less efficient form for *all* ranges,
> rather
> than only using the less efficient form for ranges that aren't "caret
> at
> start, length < 64 chars".

Indeed.

> I know you're simply following the suggestion in the PR, sorry,

Sorry.  I did put a caveat on the suggestion FWIW.

> but I wonder if the option should instead be:
> 
> -flarge-source-files
> 
> since that seems like a more user-facing concept.  The option would
> tell GCC that the source files are likely to be very large and that
> GCC should adapt accordingly.  In particular, the option makes GCC
> cope with more source lines at the expense of slowing down
> compilation
> and using more memory.

Another approach would be to go lower-level and introduce a param for
this; something like "--param location-range-bits" defaulting to 5; the
user can set it to 0 to disable the range-bit optimization to deal with
bigger files, and it allows for experimentation without rebuilding the
compiler.

Again, I don't know if this is a good idea; I'm thinking aloud; I'm not
sure what the best direction here is.

Sorry again about the belated response on this patch.
Dave


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-21 15:21     ` David Malcolm
@ 2020-04-21 18:46       ` Richard Sandiford
  2020-04-22 14:22         ` Qing Zhao
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Sandiford @ 2020-04-21 18:46 UTC (permalink / raw)
  To: David Malcolm; +Cc: Qing Zhao via Gcc-patches, jakub, Qing Zhao

David Malcolm <dmalcolm@redhat.com> writes:
> On Tue, 2020-04-21 at 15:04 +0100, Richard Sandiford wrote:
>> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> > Hi,
>> > 
>> > Please take a look at the attached patch and let me know your
>> > comments.
>> > 
>> > Thanks.
>> > 
>> > Qing
>> 
>> Acting as a reviewer of last resort since this isn't really my area,
>
> Sorry; life has been crazy lately.

NP, just thought I'd better justify treading in this area. :-)

>> but FWIW, I agree losing column tracking at the current limit is a
>> genuine regression and should be fixed for GCC 10.
>
> Is this a regression in GCC 10 though?  I think this has been the case
> since GCC 6 onwards.

Agree it's not a regression in GCC 10, but it still seems fair game
as a regression in general.

>> > gcc/ChangeLog:
>> > 
>> > 2020-04-03  qing zhao  <qing.zhao@oracle.com>
>> > 
>> 
>> Please add:
>> 
>> 	PR c/94230
>> 
>> > 	* common.opt: Add -flocation-ranges.
>> > 	* doc/invoke.texi: Document it.
>> > 	* toplev.c (process_options): set line_table-
>> > >default_range_bits
>> > 	to 0 when flag_location_ranges is false. 
>> 
>> I think it would be worth adding a hint to use the new option to
>> get_visual_column, when warning about column tracking being disabled.
>> This should probably be a second inform(), immediately after the
>> current one.
>> 
>> > @@ -14151,6 +14151,13 @@ 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 -flocation-ranges
>> > +@opindex flocation-ranges
>> 
>> Normally the documented option should be the non-default one,
>> so -fno-... in this case.
>> 
>> > +Enable range tracking when recording source locations.
>> > +By default, GCC enables range tracking when recording source
>> > locations.
>> > +If disable range tracking by -fno-location-ranges, more location
>> > space
>> > +will be saved for column tracking.
>> 
>> My understanding is that the patch doesn't actually disable location-
>> range
>> tracking, but simply uses a less efficient form for *all* ranges,
>> rather
>> than only using the less efficient form for ranges that aren't "caret
>> at
>> start, length < 64 chars".
>
> Indeed.
>
>> I know you're simply following the suggestion in the PR, sorry,
>
> Sorry.  I did put a caveat on the suggestion FWIW.
>
>> but I wonder if the option should instead be:
>> 
>> -flarge-source-files
>> 
>> since that seems like a more user-facing concept.  The option would
>> tell GCC that the source files are likely to be very large and that
>> GCC should adapt accordingly.  In particular, the option makes GCC
>> cope with more source lines at the expense of slowing down
>> compilation
>> and using more memory.
>
> Another approach would be to go lower-level and introduce a param for
> this; something like "--param location-range-bits" defaulting to 5; the
> user can set it to 0 to disable the range-bit optimization to deal with
> bigger files, and it allows for experimentation without rebuilding the
> compiler.
>
> Again, I don't know if this is a good idea; I'm thinking aloud; I'm not
> sure what the best direction here is.

The reason I like the -flarge-source-files (suggestion for better
names welcome) is that the user is giving user-level information and
letting the compiler decide how to deal with that.  What the option
actually does can change with the implementation as necessary.

Potentially any user could hit the -Wmisleading-indent note, or could
hit the limit at which columns get dropped from diagnostics.  So while
this option isn't going to be used all that often, it also doesn't feel
like an option designed specifically for “power users” who like to
experiment with compiler internals.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: PING [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-21 18:46       ` Richard Sandiford
@ 2020-04-22 14:22         ` Qing Zhao
  2020-04-23 14:41           ` [Version 2][PATCH][gcc][PR94230]provide " Qing Zhao
  0 siblings, 1 reply; 16+ messages in thread
From: Qing Zhao @ 2020-04-22 14:22 UTC (permalink / raw)
  To: Richard Sandiford, dmalcolm; +Cc: Qing Zhao via Gcc-patches, jakub

Hi, Richard And Dave:

Thanks a lot for the review and comments.

> On Apr 21, 2020, at 1:46 PM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> David Malcolm <dmalcolm@redhat.com> writes:
>> On Tue, 2020-04-21 at 15:04 +0100, Richard Sandiford wrote
>>> 
>>> Please add:
>>> 
>>> 	PR c/94230

Will do. 

>>> 
>>>> 	* common.opt: Add -flocation-ranges.
>>>> 	* doc/invoke.texi: Document it.
>>>> 	* toplev.c (process_options): set line_table-
>>>>> default_range_bits
>>>> 	to 0 when flag_location_ranges is false. 
>>> 
>>> I think it would be worth adding a hint to use the new option to
>>> get_visual_column, when warning about column tracking being disabled.
>>> This should probably be a second inform(), immediately after the
>>> current one.

Sounds reasonable to me, I will add that.

>>> 
>>>> @@ -14151,6 +14151,13 @@ 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 -flocation-ranges
>>>> +@opindex flocation-ranges
>>> 
>>> Normally the documented option should be the non-default one,
>>> so -fno-... in this case.

Okay. 

>>> 
>>>> +Enable range tracking when recording source locations.
>>>> +By default, GCC enables range tracking when recording source
>>>> locations.
>>>> +If disable range tracking by -fno-location-ranges, more location
>>>> space
>>>> +will be saved for column tracking.
>>> 
>>> My understanding is that the patch doesn't actually disable location-
>>> range
>>> tracking, but simply uses a less efficient form for *all* ranges,
>>> rather
>>> than only using the less efficient form for ranges that aren't "caret
>>> at
>>> start, length < 64 chars".
>> 
>> Indeed.

Okay, I see. 
Providing a good documentation at the user level for this option might be a challenge to me, I will try.  -:)

>> 
>>> I know you're simply following the suggestion in the PR, sorry,
>> 
>> Sorry.  I did put a caveat on the suggestion FWIW.
>> 
>>> but I wonder if the option should instead be:
>>> 
>>> -flarge-source-files
>>> 
>>> since that seems like a more user-facing concept.  The option would
>>> tell GCC that the source files are likely to be very large and that
>>> GCC should adapt accordingly.  In particular, the option makes GCC
>>> cope with more source lines at the expense of slowing down
>>> compilation
>>> and using more memory.
>> 
>> Another approach would be to go lower-level and introduce a param for
>> this; something like "--param location-range-bits" defaulting to 5; the
>> user can set it to 0 to disable the range-bit optimization to deal with
>> bigger files, and it allows for experimentation without rebuilding the
>> compiler.
>> 
>> Again, I don't know if this is a good idea; I'm thinking aloud; I'm not
>> sure what the best direction here is.
> 
> The reason I like the -flarge-source-files (suggestion for better
> names welcome) is that the user is giving user-level information and
> letting the compiler decide how to deal with that.  What the option
> actually does can change with the implementation as necessary.
> 
> Potentially any user could hit the -Wmisleading-indent note, or could
> hit the limit at which columns get dropped from diagnostics.  So while
> this option isn't going to be used all that often, it also doesn't feel
> like an option designed specifically for “power users” who like to
> experiment with compiler internals.

Agreed, I prefer to use -flarge-source-files for this functionality. 

Let me know if you have any other suggestions for this patch.

Thanks.

Qing


> 
> Thanks,
> Richard


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Version 2][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-22 14:22         ` Qing Zhao
@ 2020-04-23 14:41           ` Qing Zhao
  2020-04-23 18:27             ` Richard Sandiford
  0 siblings, 1 reply; 16+ messages in thread
From: Qing Zhao @ 2020-04-23 14:41 UTC (permalink / raw)
  To: Richard Sandiford, dmalcolm; +Cc: Qing Zhao via Gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]

Hi, 
This is the second version of the patch based on the previous discussion.
In this new version, the major changes are:

1. The name of the option is changed to -flarge-source-files;
2. Add a hint to use this new option “-flarge-source-files” in the routine “get_visual_column”;
3. Documentation for this new option;
4. Update the testing case location-overflow-test-1.c to include the new hint.

Please take a look at this new patch and let me know any new comment.

thanks.

Qing.

gcc/ChangeLog:

2020-04-22  qing zhao  <qing.zhao@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.zhao@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.zhao@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.


[-- Attachment #2: PR94230.patch --]
[-- Type: application/octet-stream, Size: 4355 bytes --]

From dad549b13e608aede097aebfe6b257f9b10b843b Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Wed, 22 Apr 2020 15:31:17 -0400
Subject: [PATCH] Fix PR94230

---
 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;
     }
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.
+
 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
@@ -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.
+
 @end table
 
 @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'" } */
   return x * y;
 }
 
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 4c8be50..b3230b7 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1854,6 +1854,9 @@ process_options (void)
     hash_table_sanitize_eq_limit
       = param_hash_table_verification_limit;
 
+  if (flag_large_source_files)
+    line_table->default_range_bits = 0;
+
   /* Please don't change global_options after this point, those changes won't
      be reflected in optimization_{default,current}_node.  */
 }
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 4008 bytes --]





> On Apr 22, 2020, at 9:22 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> Hi, Richard And Dave:
> 
> Thanks a lot for the review and comments.
> 
>> On Apr 21, 2020, at 1:46 PM, Richard Sandiford <richard.sandiford@arm.com> wrote:
>> 
>> David Malcolm <dmalcolm@redhat.com> writes:
>>> On Tue, 2020-04-21 at 15:04 +0100, Richard Sandiford wrote
>>>> 
>>>> Please add:
>>>> 
>>>> 	PR c/94230
> 
> Will do. 
> 
>>>> 
>>>>> 	* common.opt: Add -flocation-ranges.
>>>>> 	* doc/invoke.texi: Document it.
>>>>> 	* toplev.c (process_options): set line_table-
>>>>>> default_range_bits
>>>>> 	to 0 when flag_location_ranges is false. 
>>>> 
>>>> I think it would be worth adding a hint to use the new option to
>>>> get_visual_column, when warning about column tracking being disabled.
>>>> This should probably be a second inform(), immediately after the
>>>> current one.
> 
> Sounds reasonable to me, I will add that.
> 
>>>> 
>>>>> @@ -14151,6 +14151,13 @@ 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 -flocation-ranges
>>>>> +@opindex flocation-ranges
>>>> 
>>>> Normally the documented option should be the non-default one,
>>>> so -fno-... in this case.
> 
> Okay. 
> 
>>>> 
>>>>> +Enable range tracking when recording source locations.
>>>>> +By default, GCC enables range tracking when recording source
>>>>> locations.
>>>>> +If disable range tracking by -fno-location-ranges, more location
>>>>> space
>>>>> +will be saved for column tracking.
>>>> 
>>>> My understanding is that the patch doesn't actually disable location-
>>>> range
>>>> tracking, but simply uses a less efficient form for *all* ranges,
>>>> rather
>>>> than only using the less efficient form for ranges that aren't "caret
>>>> at
>>>> start, length < 64 chars".
>>> 
>>> Indeed.
> 
> Okay, I see. 
> Providing a good documentation at the user level for this option might be a challenge to me, I will try.  -:)
> 
>>> 
>>>> I know you're simply following the suggestion in the PR, sorry,
>>> 
>>> Sorry.  I did put a caveat on the suggestion FWIW.
>>> 
>>>> but I wonder if the option should instead be:
>>>> 
>>>> -flarge-source-files
>>>> 
>>>> since that seems like a more user-facing concept.  The option would
>>>> tell GCC that the source files are likely to be very large and that
>>>> GCC should adapt accordingly.  In particular, the option makes GCC
>>>> cope with more source lines at the expense of slowing down
>>>> compilation
>>>> and using more memory.
>>> 
>>> Another approach would be to go lower-level and introduce a param for
>>> this; something like "--param location-range-bits" defaulting to 5; the
>>> user can set it to 0 to disable the range-bit optimization to deal with
>>> bigger files, and it allows for experimentation without rebuilding the
>>> compiler.
>>> 
>>> Again, I don't know if this is a good idea; I'm thinking aloud; I'm not
>>> sure what the best direction here is.
>> 
>> The reason I like the -flarge-source-files (suggestion for better
>> names welcome) is that the user is giving user-level information and
>> letting the compiler decide how to deal with that.  What the option
>> actually does can change with the implementation as necessary.
>> 
>> Potentially any user could hit the -Wmisleading-indent note, or could
>> hit the limit at which columns get dropped from diagnostics.  So while
>> this option isn't going to be used all that often, it also doesn't feel
>> like an option designed specifically for “power users” who like to
>> experiment with compiler internals.
> 
> Agreed, I prefer to use -flarge-source-files for this functionality. 
> 
> Let me know if you have any other suggestions for this patch.
> 
> Thanks.
> 
> Qing
> 
> 
>> 
>> Thanks,
>> Richard
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Version 2][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-23 14:41           ` [Version 2][PATCH][gcc][PR94230]provide " Qing Zhao
@ 2020-04-23 18:27             ` Richard Sandiford
  2020-04-23 19:52               ` Qing Zhao
  2020-04-23 21:05               ` [Version 3][PATCH][gcc][PR94230]provide " Qing Zhao
  0 siblings, 2 replies; 16+ messages in thread
From: Richard Sandiford @ 2020-04-23 18:27 UTC (permalink / raw)
  To: Qing Zhao; +Cc: dmalcolm, Qing Zhao via Gcc-patches

Qing Zhao <QING.ZHAO@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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Version 2][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-23 18:27             ` Richard Sandiford
@ 2020-04-23 19:52               ` Qing Zhao
  2020-04-23 21:05               ` [Version 3][PATCH][gcc][PR94230]provide " Qing Zhao
  1 sibling, 0 replies; 16+ messages in thread
From: Qing Zhao @ 2020-04-23 19:52 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: dmalcolm, Qing Zhao via Gcc-patches

Hi, Richard,


> On Apr 23, 2020, at 1:27 PM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> Qing Zhao <QING.ZHAO@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.

Yes, indeed, will add it.
> 
>> 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.
> ———————————

Sounds better than my previous wording. Thanks.

> 
>> 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.

Okay. 
> 
>> @@ -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 a lot for this paragraph. I will use it.

Qing
> 
> Thanks,
> Richard


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Version 3][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-23 18:27             ` Richard Sandiford
  2020-04-23 19:52               ` Qing Zhao
@ 2020-04-23 21:05               ` Qing Zhao
  2020-04-23 22:13                 ` David Malcolm
  1 sibling, 1 reply; 16+ messages in thread
From: Qing Zhao @ 2020-04-23 21:05 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: dmalcolm, Qing Zhao via Gcc-patches

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

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.zhao@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.zhao@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.zhao@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.


[-- Attachment #2: PR94230.patch --]
[-- Type: application/octet-stream, Size: 4734 bytes --]

From 97696d2c1c146fd7d3161f592d42cd7d73c6d5a8 Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@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");
 	}
       return false;
     }
diff --git a/gcc/common.opt b/gcc/common.opt
index 4368910..03fd4be 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)
+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..cd580ab 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -573,8 +573,8 @@ Objective-C and Objective-C++ Dialects}.
 -dD  -dI  -dM  -dN  -dU @gol
 -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
+-finput-charset=@var{charset}  -flarge-source-files  @gol
+-fmacro-prefix-map=@var{old}=@var{new} -fmax-include-depth=@var{depth} @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
@@ -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
+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.
+
 @end table
 
 @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'" } */
   return x * y;
 }
 
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 4c8be50..b3230b7 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1854,6 +1854,9 @@ process_options (void)
     hash_table_sanitize_eq_limit
       = param_hash_table_verification_limit;
 
+  if (flag_large_source_files)
+    line_table->default_range_bits = 0;
+
   /* Please don't change global_options after this point, those changes won't
      be reflected in optimization_{default,current}_node.  */
 }
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 4754 bytes --]



> On Apr 23, 2020, at 1:27 PM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> Qing Zhao <QING.ZHAO@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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Version 3][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-23 21:05               ` [Version 3][PATCH][gcc][PR94230]provide " Qing Zhao
@ 2020-04-23 22:13                 ` David Malcolm
  2020-04-24 22:22                   ` [Version 4][PATCH][gcc][PR94230]provide " Qing Zhao
  2020-05-06 18:40                   ` Committed [Version 3][PATCH][gcc][PR94230]provide " Qing Zhao
  0 siblings, 2 replies; 16+ messages in thread
From: David Malcolm @ 2020-04-23 22:13 UTC (permalink / raw)
  To: Qing Zhao, Richard Sandiford, jakub, Richard Biener
  Cc: Qing Zhao via Gcc-patches

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.zhao@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.zhao@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.zhao@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.sandiford@arm.com> wrote:
> > 
> > Qing Zhao <QING.ZHAO@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.zhao@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.zhao@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.zhao@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.zhao@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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Version 4][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-23 22:13                 ` David Malcolm
@ 2020-04-24 22:22                   ` Qing Zhao
  2020-04-24 22:36                     ` David Malcolm
  2020-05-06 18:40                   ` Committed [Version 3][PATCH][gcc][PR94230]provide " Qing Zhao
  1 sibling, 1 reply; 16+ messages in thread
From: Qing Zhao @ 2020-04-24 22:22 UTC (permalink / raw)
  To: David Malcolm, Richard Sandiford, Jakub, richard Biener
  Cc: Qing Zhao via Gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

Hi, Dave,

Thanks a lot for the review and comments.
I just updated the patch with all your suggestions, bootstrapped it and run regression test, no any issue.

The newest patch is attached with this email.

Richard/Jakub, please advise on whether I can commit this patch to Gcc10?

Thanks a lot.

Qing

gcc/c-family/ChangeLog:

2020-04-24  qing zhao  <qing.zhao@oracle.com>

        * c-indentation.c (get_visual_column): Add a hint to use the new
        -flarge-source-files option.

gcc/ChangeLog:

2020-04-24  qing zhao  <qing.zhao@oracle.com>

        * 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/testsuite/ChangeLog:

2020-04-24  qing zhao  <qing.zhao@oracle.com>

        * gcc.dg/plugin/location-overflow-test-1.c (fn_1): New message to
        verify that hint about -flarge-source-files is emitted.


[-- Attachment #2: PR94230.patch --]
[-- Type: application/octet-stream, Size: 4820 bytes --]

From 512a8ccce1e6ed8889cd7144459c130935abf0de Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Fri, 24 Apr 2020 16:50:18 +0200
Subject: [PATCH] Fix PR94230

---
 gcc/c-family/c-indentation.c                          |  5 +++++
 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, 31 insertions(+), 3 deletions(-)

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index f737555db0e..baa506347cd 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -67,6 +67,11 @@ 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,
+		    "adding %<-flarge-source-files%> will allow for more" 
+		    " column-tracking support, at the expense of compilation"
+		    " and memory");
 	}
       return false;
     }
diff --git a/gcc/common.opt b/gcc/common.opt
index d33383b523c..30d05734d16 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1609,6 +1609,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)
+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 a37a2ee9c19..bdde7c1d881 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -574,8 +574,8 @@ Objective-C and Objective-C++ Dialects}.
 -dD  -dI  -dM  -dN  -dU @gol
 -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
+-finput-charset=@var{charset}  -flarge-source-files  @gol
+-fmacro-prefix-map=@var{old}=@var{new} -fmax-include-depth=@var{depth} @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
@@ -14183,6 +14183,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-indentation} 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 columns.
+
 @end table
 
 @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 1a80a668a0f..a3ac12d3b46 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" "adding '-flarge-source-files'" } */
   return x * y;
 }
 
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 4c8be502c71..b3230b71b9d 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1854,6 +1854,9 @@ process_options (void)
     hash_table_sanitize_eq_limit
       = param_hash_table_verification_limit;
 
+  if (flag_large_source_files)
+    line_table->default_range_bits = 0;
+
   /* Please don't change global_options after this point, those changes won't
      be reflected in optimization_{default,current}_node.  */
 }
-- 
2.11.0

[-- Attachment #3: Type: text/plain, Size: 2138 bytes --]



> On Apr 23, 2020, at 5:13 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> 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.
> 
> 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.
> 
> 
> Maybe: "Verify that hint about -flarge-source-files is emitted".
> 
> 
>> +	  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.
> 
>> +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
> 
>> +  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
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Version 4][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-24 22:22                   ` [Version 4][PATCH][gcc][PR94230]provide " Qing Zhao
@ 2020-04-24 22:36                     ` David Malcolm
  2020-04-27 14:23                       ` Qing Zhao
  0 siblings, 1 reply; 16+ messages in thread
From: David Malcolm @ 2020-04-24 22:36 UTC (permalink / raw)
  To: Qing Zhao, Richard Sandiford, Jakub, richard Biener
  Cc: Qing Zhao via Gcc-patches

On Fri, 2020-04-24 at 17:22 -0500, Qing Zhao wrote:
> Hi, Dave,
> 
> Thanks a lot for the review and comments.
> I just updated the patch with all your suggestions, bootstrapped it
> and run regression test, no any issue.
> 
> The newest patch is attached with this email.
> 
> Richard/Jakub, please advise on whether I can commit this patch to
> Gcc10?
> 
> Thanks a lot.
> 
> Qing
> 

Thanks Qing.  One more wording nit (sorry!)

> +	  if (!flag_large_source_files)
> +	    inform (loc,
> +		    "adding %<-flarge-source-files%> will allow for more" 
> +		    " column-tracking support, at the expense of compilation"
> +		    " and memory");
                    ^^^^^^^^^^^^^
Please add "time" here i.e.
                    " time and memory");


Dave


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Version 4][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-24 22:36                     ` David Malcolm
@ 2020-04-27 14:23                       ` Qing Zhao
  0 siblings, 0 replies; 16+ messages in thread
From: Qing Zhao @ 2020-04-27 14:23 UTC (permalink / raw)
  To: David Malcolm
  Cc: Richard Sandiford, Jakub, richard Biener, Qing Zhao via Gcc-patches

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

Hi, David,

> On Apr 24, 2020, at 5:36 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> On Fri, 2020-04-24 at 17:22 -0500, Qing Zhao wrote:
>> Hi, Dave,
>> 
>> Thanks a lot for the review and comments.
>> I just updated the patch with all your suggestions, bootstrapped it
>> and run regression test, no any issue.
>> 
>> The newest patch is attached with this email.
>> 
>> Richard/Jakub, please advise on whether I can commit this patch to
>> Gcc10?
>> 
>> Thanks a lot.
>> 
>> Qing
>> 
> 
> Thanks Qing.  One more wording nit (sorry!)
> 
>> +	  if (!flag_large_source_files)
>> +	    inform (loc,
>> +		    "adding %<-flarge-source-files%> will allow for more" 
>> +		    " column-tracking support, at the expense of compilation"
>> +		    " and memory");
>                    ^^^^^^^^^^^^^
> Please add "time" here i.e.
>                    " time and memory”);

Added it in my local directory. (And attached).

Thanks.

Qing

[-- Attachment #2: PR94230.patch --]
[-- Type: application/octet-stream, Size: 4825 bytes --]

From 9f8e4e351be1f6310c8bcd868f5ef71f8d210875 Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Mon, 27 Apr 2020 16:20:19 +0200
Subject: [PATCH] Fix PR94230

---
 gcc/c-family/c-indentation.c                          |  5 +++++
 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, 31 insertions(+), 3 deletions(-)

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index f737555db0e..9fba3bcc67c 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -67,6 +67,11 @@ 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,
+		    "adding %<-flarge-source-files%> will allow for more" 
+		    " column-tracking support, at the expense of compilation"
+		    " time and memory");
 	}
       return false;
     }
diff --git a/gcc/common.opt b/gcc/common.opt
index d33383b523c..30d05734d16 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1609,6 +1609,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)
+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 a37a2ee9c19..bdde7c1d881 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -574,8 +574,8 @@ Objective-C and Objective-C++ Dialects}.
 -dD  -dI  -dM  -dN  -dU @gol
 -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
+-finput-charset=@var{charset}  -flarge-source-files  @gol
+-fmacro-prefix-map=@var{old}=@var{new} -fmax-include-depth=@var{depth} @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
@@ -14183,6 +14183,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-indentation} 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 columns.
+
 @end table
 
 @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 1a80a668a0f..a3ac12d3b46 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" "adding '-flarge-source-files'" } */
   return x * y;
 }
 
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 4c8be502c71..b3230b71b9d 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1854,6 +1854,9 @@ process_options (void)
     hash_table_sanitize_eq_limit
       = param_hash_table_verification_limit;
 
+  if (flag_large_source_files)
+    line_table->default_range_bits = 0;
+
   /* Please don't change global_options after this point, those changes won't
      be reflected in optimization_{default,current}_node.  */
 }
-- 
2.11.0

[-- Attachment #3: Type: text/plain, Size: 20 bytes --]




> 
> 
> Dave
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Committed [Version 3][PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent
  2020-04-23 22:13                 ` David Malcolm
  2020-04-24 22:22                   ` [Version 4][PATCH][gcc][PR94230]provide " Qing Zhao
@ 2020-05-06 18:40                   ` Qing Zhao
  1 sibling, 0 replies; 16+ messages in thread
From: Qing Zhao @ 2020-05-06 18:40 UTC (permalink / raw)
  To: David Malcolm; +Cc: Richard Sandiford, Qing Zhao via Gcc-patches

FYI.

> On Apr 23, 2020, at 5:13 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> May need updating to match the hint message.
> 
> [...]
> 
> This is OK for stage 1 with those nits fixed.

I committed the updated patch with all the suggestions today to gcc11 stage1 as:

https://gcc.gnu.org/g:530b44094354758d0dea5374188caa6863647114 <https://gcc.gnu.org/g:530b44094354758d0dea5374188caa6863647114>

Thanks.

Qing

> 
> 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
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-05-06 18:42 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 19:55 [PATCH][gcc][PR94230]provide an option to change the size limitation for -Wmisleading-indent Qing Zhao
2020-04-08 19:39 ` PING " Qing Zhao
2020-04-15 20:30   ` Fwd: " Qing Zhao
2020-04-21 14:04   ` Richard Sandiford
2020-04-21 15:21     ` David Malcolm
2020-04-21 18:46       ` Richard Sandiford
2020-04-22 14:22         ` Qing Zhao
2020-04-23 14:41           ` [Version 2][PATCH][gcc][PR94230]provide " Qing Zhao
2020-04-23 18:27             ` Richard Sandiford
2020-04-23 19:52               ` Qing Zhao
2020-04-23 21:05               ` [Version 3][PATCH][gcc][PR94230]provide " Qing Zhao
2020-04-23 22:13                 ` David Malcolm
2020-04-24 22:22                   ` [Version 4][PATCH][gcc][PR94230]provide " Qing Zhao
2020-04-24 22:36                     ` David Malcolm
2020-04-27 14:23                       ` Qing Zhao
2020-05-06 18:40                   ` Committed [Version 3][PATCH][gcc][PR94230]provide " Qing Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).