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

[Alexandre Oliva <aoliva@redhat.com>]

On Feb  9, 2018, Jakub Jelinek <jakub@redhat.com> wrote:

> On Fri, Feb 09, 2018 at 07:01:25PM -0200, Alexandre Oliva wrote:
>> 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?

> Better in the default version of the target hook check the param
> whether it should return true or false, and for analyzed targets
> just use an always true (or false, depending on what the hook is)
> as the hook.

I want it to be overridable, so here's what I ended up with.
Testing underway; ok to install if it succeeds?


We need accurate length information to be able to optimize away
locviews that would contain only zero-numbered views, and even to
output correct view numbers.  E.g., if we assume an insn has length
nonzero, i.e., that it will advance PC, we conclude the view number
after it is zero.  If the assumption turns out to be false, the
assembler view zero assert will catch the mistake, but only if we're
using a view-capable assembler.  Otherwise, view numbers will silently
go out of sync between the compiler and the assembler, and thus the
debug info consumer.

Unfortunately, a number of targets seem to fail the essential property
for this to work, namely, that get_min_attr_length() be zero for every
insn that might have length zero.

We thus introduce a target hook that indicates whether we can rely on
the length attribute for this purpose, and a param that can be used to
override it, for testing purposes or to work around errors.

for  gcc/ChangeLog

	* params.def (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING): New.
	* target.def (attr_length_reliable_for_view_count): New.
	* dwarf2out.c: Include params.h.
	(unreliable_attr_length_p): New.
	(dwarf2out_var_location): Use it.
	* doc/tm.texi.in (TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT):
	New.
	* doc/tm.texi: Rebuilt.
---
 gcc/doc/tm.texi    |   13 +++++++++++++
 gcc/doc/tm.texi.in |    2 ++
 gcc/dwarf2out.c    |   28 +++++++++++++++++++++++++++-
 gcc/params.def     |    5 +++++
 gcc/target.def     |   14 ++++++++++++++
 5 files changed, 61 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..c48c117c8a16 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);
@@ -26860,6 +26861,30 @@ dwarf2out_next_real_insn (rtx_insn *loc_note)
   return next_real;
 }
 
+/* Return TRUE if we should NOT use ATTR_length to optimize locview
+   lists, and FALSE otherwise.
+
+   If ATTR_length is always zero for insns that could have length
+   zero, we can use it, then we'll know when we're guaranteed to have
+   a PC change that implies a view reset.  locview lists whose views
+   are all zero can be optimized out completely.
+
+   However, if we enable these optimizations based on unreliable data,
+   we might get errors from the assembler (binutils 2.30+) when it
+   notices we have incorrectly assumed a view would be zero, or
+   silently get incorrect locview lists otherwise.  */
+
+static bool
+unreliable_attr_length_p (void)
+{
+  int parm = PARAM_VALUE (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING);
+
+  if (parm != -1)
+    return !parm;
+  else
+    return !targetm.attr_length_reliable_for_view_count;
+}
+
 /* Called by the final INSN scan whenever we see a var location.  We
    use it to drop labels in the right places, and throw the location in
    our lookup table.  */
@@ -26921,7 +26946,8 @@ dwarf2out_var_location (rtx_insn *loc_note)
 	gcc_unreachable ();
       else if (JUMP_TABLE_DATA_P (loc_note))
 	RESET_NEXT_VIEW (cur_line_info_table->view);
-      else if (GET_CODE (loc_note) == USE
+      else if (unreliable_attr_length_p ()
+	       || GET_CODE (loc_note) == USE
 	       || GET_CODE (loc_note) == CLOBBER
 	       || GET_CODE (loc_note) == ASM_INPUT
 	       || asm_noperands (loc_note) >= 0)
diff --git a/gcc/params.def b/gcc/params.def
index 930b31820be9..12e217ed9da4 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.",
+	 -1 /* targetm.attr_length_reliable_for_view_count */, 0, 1)
+
 /*
 
 Local variables:
diff --git a/gcc/target.def b/gcc/target.def
index aeb41df19454..a7fedb72fef2 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -6631,6 +6631,20 @@ 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, during final, get_min_attr_length returns zero\n\
+for all non-asm insns that might have length zero.\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