Re: [SFN+LVU+IEPM v4 9/9] [IEPM] Introduce inline entry point markers

[Alexandre Oliva <aoliva@redhat.com>]

On Feb  9, 2018, Jeff Law <law@redhat.com> wrote:

> On 02/08/2018 08:53 PM, Alan Modra wrote:
>> On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote:
>>> Here's what I checked in, right after the LVU patch.
>>> 
>>> [IEPM] Introduce inline entry point markers
>> 
>> One of these two patches breaks ppc64le bootstrap with the assembler
>> complaining "Error: view number mismatch" when compiling
>> libdecnumber.
>> 
> I've just passed along a similar failure (.i, .s and command line
> options) to Alex for ppc64 (be) building glibc.

Thanks.  So, I'm told there are more such issues, that non-asm insn
length attrs can't be relied on at this time to be nonzero only when the
actual length is not zero.  When we fail that and regard a zero-length
insn as nonzero and that's all we have between two subsequent views, the
assembler (GNU as 2.30) will catch and report the error.  With other
assemblers, the incorrect view reset will go unnoticed and result in
incorrect debug info.

So, as discussed on IRC, I'm trying to use a target hook to allow
targets to indicate that their length attrs have been assessed for this
purpose, and a param to make that overridable, but I'm having trouble
initializing the param from the target hook.  How does one do that?

Meanwhile, here's the (WIP) patch that introduces the param defaulting
to no locview optimizations (which can only be performed at points in
which the compiler knows there are PC changes); I'd like it to default
to targetm.attr_length_reliable_for_view_count, but I couldn't figure
out how to do so.  Help?


By disabling it altogether, we won't get the assembler checks or
incorrect view numbers in debug info, but we will get plenty of all-zero
locview lists.  Oh well...  I guess at this point that's better than
wrong debug info or assembler failures.




disable locview optimizations for now

---
 gcc/doc/tm.texi    |   13 +++++++++++++
 gcc/doc/tm.texi.in |    2 ++
 gcc/dwarf2out.c    |    4 +++-
 gcc/opts.c         |    6 ++++++
 gcc/params.def     |    5 +++++
 gcc/target.def     |   13 +++++++++++++
 6 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index ddf48cb4b4d2..50fa0d387f32 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -9986,6 +9986,19 @@ following it should not be run.  Usually true only for virtual assembler
 targets.
 @end deftypevr
 
+@deftypevr {Target Hook} bool TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT
+TRUE if get_min_attr_length returns
+zero for any non-asm insn that might have length zero, during final.
+It's ok if it's conservative and always returns zero.
+If, in addition to the essential property, when an insn is known to have
+nonzero length, get_min_attr_length returns a positive number, that will
+enable loclist optimizations.  There is little point in making this TRUE
+otherwise.  Enabling this when the essential property is not met while
+using GNU as 2.30 or newer may cause the assembler to detect the errors
+and fail to assemble; other assemblers will silently let the errors
+through, with incorrect view numbers in debug information.
+@end deftypevr
+
 @defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2})
 A C statement to issue assembly directives that create a difference
 @var{lab1} minus @var{lab2}, using an integer of the given @var{size}.
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 0aab45f4992c..26b32db77f74 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -6929,6 +6929,8 @@ tables, and hence is desirable if it works.
 
 @hook TARGET_NO_REGISTER_ALLOCATION
 
+@hook TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT
+
 @defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2})
 A C statement to issue assembly directives that create a difference
 @var{lab1} minus @var{lab2}, using an integer of the given @var{size}.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 749c7e3b9bbc..6a6520a05e8b 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -96,6 +96,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stringpool.h"
 #include "attribs.h"
 #include "file-prefix-map.h" /* remap_debug_filename()  */
+#include "params.h"
 
 static void dwarf2out_source_line (unsigned int, unsigned int, const char *,
 				   int, bool);
@@ -26926,7 +26927,8 @@ dwarf2out_var_location (rtx_insn *loc_note)
 	       || GET_CODE (loc_note) == ASM_INPUT
 	       || asm_noperands (loc_note) >= 0)
 	;
-      else if (get_attr_min_length (loc_note) > 0)
+      else if (PARAM_VALUE (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING)
+	       && get_attr_min_length (loc_note) > 0)
 	RESET_NEXT_VIEW (cur_line_info_table->view);
 
       return;
diff --git a/gcc/opts.c b/gcc/opts.c
index f2795f98bf44..d4b3065caad6 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1039,6 +1039,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
   if ((opts->x_flag_sanitize & SANITIZE_KERNEL_ADDRESS) && opts->x_flag_tm)
     sorry ("transactional memory is not supported with "
 	   "%<-fsanitize=kernel-address%>");
+
+#if 0
+  maybe_set_param_value (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING,
+			 targetm.attr_length_reliable_for_view_count,
+			 opts->x_param_values, opts_set->x_param_values);
+#endif
 }
 
 #define LEFT_COLUMN	27
diff --git a/gcc/params.def b/gcc/params.def
index 930b31820be9..184e435c786a 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1331,6 +1331,11 @@ DEFPARAM(PARAM_AVOID_FMA_MAX_BITS,
 	 "Maximum number of bits for which we avoid creating FMAs.",
 	 0, 0, 512)
 
+DEFPARAM(PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING,
+	 "use-attr-length-in-view-counting",
+	 "Optimize locview lists based on length attributes.",
+	 0 /* targetm.attr_length_reliable_for_view_count */, 0, 1)
+
 /*
 
 Local variables:
diff --git a/gcc/target.def b/gcc/target.def
index aeb41df19454..6966053157a3 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -6631,6 +6631,19 @@ following it should not be run.  Usually true only for virtual assembler\n\
 targets.",
 bool, false)
 
+DEFHOOKPOD
+(attr_length_reliable_for_view_count, "TRUE if get_min_attr_length returns\n\
+zero for any non-asm insn that might have length zero, during final.\n\
+It's ok if it's conservative and always returns zero.\n\
+If, in addition to the essential property, when an insn is known to have\n\
+nonzero length, get_min_attr_length returns a positive number, that will\n\
+enable loclist optimizations.  There is little point in making this TRUE\n\
+otherwise.  Enabling this when the essential property is not met while\n\
+using GNU as 2.30 or newer may cause the assembler to detect the errors\n\
+and fail to assemble; other assemblers will silently let the errors\n\
+through, with incorrect view numbers in debug information.",
+bool, false)
+
 /* Leave the boolean fields at the end.  */
 
 /* Functions related to mode switching.  */


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer