Re: [PATCH] Add comments to struct cgraph_thunk_info

[Pierre-Marie de Rodat <derodat at adacore dot com>]

On 09/15/2017 12:54 PM, Pierre-Marie de Rodat wrote:
> I’m not super confident about this though, so I’ll resubmit a patch 
> without the reordering: I’ve added more comments anyway as I’ve learned 
> more about this since yesterday. ;-)

Here it is!

--
Pierre-Marie de Rodat

>From 601dd0e949f4af456a11036918da9dbadbb3aa0c Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Wed, 13 Sep 2017 16:21:04 +0200
Subject: [PATCH] Add comments to struct cgraph_thunk_info

This commit adds comments to fields in the cgraph_thunk_info structure
declaration from cgraph.h.  They will hopefully answer questions that
people like myself can ask while discovering the thunk machinery.  I
also made an assertion stricter in cgraph_node::create_thunk.

Bootsrapped and regtested on x86_64-linux.

gcc/

	* cgraph.h (cgraph_thunk_info): Add comments.
	* cgraph.c (cgraph_node::create_thunk): Adjust comment, make
	assert for VIRTUAL_* arguments stricter.
---
 gcc/cgraph.c | 14 +++++++++-----
 gcc/cgraph.h | 39 +++++++++++++++++++++++++++++++++++----
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 69aa6c5bce2..8bffdec8fb7 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -603,7 +603,7 @@ cgraph_node::create_same_body_alias (tree alias, tree decl)
 
 /* Add thunk alias into callgraph.  The alias declaration is ALIAS and it
    aliases DECL with an adjustments made into the first parameter.
-   See comments in thunk_adjust for detail on the parameters.  */
+   See comments in struct cgraph_thunk_info for detail on the parameters.  */
 
 cgraph_node *
 cgraph_node::create_thunk (tree alias, tree, bool this_adjusting,
@@ -619,13 +619,17 @@ cgraph_node::create_thunk (tree alias, tree, bool this_adjusting,
     node->reset ();
   else
     node = cgraph_node::create (alias);
-  gcc_checking_assert (!virtual_offset
-		       || wi::eq_p (virtual_offset, virtual_value));
+
+  /* Make sure that if VIRTUAL_OFFSET is in sync with VIRTUAL_VALUE.  */
+  gcc_checking_assert (virtual_offset
+		       ? wi::eq_p (virtual_offset, virtual_value)
+		       : virtual_value == 0);
+
   node->thunk.fixed_offset = fixed_offset;
-  node->thunk.this_adjusting = this_adjusting;
   node->thunk.virtual_value = virtual_value;
-  node->thunk.virtual_offset_p = virtual_offset != NULL;
   node->thunk.alias = real_alias;
+  node->thunk.this_adjusting = this_adjusting;
+  node->thunk.virtual_offset_p = virtual_offset != NULL;
   node->thunk.thunk_p = true;
   node->definition = true;
 
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 57cdaa45681..c668b37ef82 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -629,17 +629,48 @@ extern const char * const cgraph_availability_names[];
 extern const char * const ld_plugin_symbol_resolution_names[];
 extern const char * const tls_model_names[];
 
-/* Information about thunk, used only for same body aliases.  */
+/* Sub-structure of cgraph_node.  Holds information about thunk, used only for
+   same body aliases.
+
+   Thunks are basically wrappers around methods which are introduced in case
+   of multiple inheritance in order to adjust the value of the "this" pointer
+   or of the returned value.
+
+   In the case of this-adjusting thunks, each back-end can override the
+   can_output_mi_thunk/output_mi_thunk target hooks to generate a minimal thunk
+   (with a tail call for instance) directly as assembly.  For the default hook
+   or for the case where the can_output_mi_thunk hooks return false, the thunk
+   is gimplified and lowered using the regular machinery.  */
 
 struct GTY(()) cgraph_thunk_info {
-  /* Information about the thunk.  */
+  /* Offset used to adjust "this".  */
   HOST_WIDE_INT fixed_offset;
+
+  /* Offset in the virtual table to get the offset to adjust "this".  Valid iff
+     VIRTUAL_OFFSET_P is true.  */
   HOST_WIDE_INT virtual_value;
+
+  /* Thunk target, i.e. the method that this thunk wraps.  Depending on the
+     TARGET_USE_LOCAL_THUNK_ALIAS_P macro, this may have to be a new alias.  */
   tree alias;
+
+  /* Nonzero for a "this" adjusting thunk and zero for a result adjusting
+     thunk.  */
   bool this_adjusting;
+
+  /* If true, this thunk is what we call a virtual thunk.  In this case:
+     * for this-adjusting thunks, after the FIXED_OFFSET based adjustment is
+       done, add to the result the offset found in the vtable at:
+	 vptr + VIRTUAL_VALUE
+     * for result-adjusting thinks, the FIXED_OFFSET adjustment is done after
+       the virtual one.  */
   bool virtual_offset_p;
+
+  /* ??? True for special kind of thunks, seems related to instrumentation.  */
   bool add_pointer_bounds_args;
-  /* Set to true when alias node is thunk.  */
+
+  /* Set to true when alias node (the cgraph_node to which this struct belong)
+     is a thunk.  Access to any other fields is invalid if this is false.  */
   bool thunk_p;
 };
 
@@ -983,7 +1014,7 @@ public:
 
   /* Add thunk alias into callgraph.  The alias declaration is ALIAS and it
      aliases DECL with an adjustments made into the first parameter.
-     See comments in thunk_adjust for detail on the parameters.  */
+     See comments in struct cgraph_thunk_info for detail on the parameters.  */
   cgraph_node * create_thunk (tree alias, tree, bool this_adjusting,
 			      HOST_WIDE_INT fixed_offset,
 			      HOST_WIDE_INT virtual_value,
-- 
2.14.1