improve internals documentation for nested function descriptors

Apropos of the discussion about improving the docs for 
TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port 
submission,

https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html

here is the patch I've come up with based on reading the source.  Is 
this technically accurate?  Any suggestions on how to improve it further?

-Sandra

> On Jul 27, 2018, at 4:39 PM, Sandra Loosemore <sandra@codesourcery.com> wrote:
> 
> Apropos of the discussion about improving the docs for TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port submission,
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html
> 
> here is the patch I've come up with based on reading the source.  Is this technically accurate?  Any suggestions on how to improve it further?
> 
> -Sandra
> <descriptor.log><descriptor.patch>

Nice.  That tells me a lot more, so I hope to hear confirmation it's accurate.

A nit: "word aligned" -- you used thas as a synonym for "multiple of 4 aligned" but some of us have 2-byte words.

On the function descriptor concept generally: it wasn't clear to me whether a target that wants to use this mechanism needs to do anything to tell GCC how to construct descriptors.  Does GCC have a standard recipe it uses that "should" work for any target?  I assume so because there aren't any other target hooks or special patterns for this.  It would be helpful to say so explicitly.

	paul

> Apropos of the discussion about improving the docs for
> TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port
> submission,
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html
> 
> here is the patch I've come up with based on reading the source.  Is
> this technically accurate?  Any suggestions on how to improve it further?

""Define this hook to 0 if the target implements custom support"

"custom" was precisely chosen to distinguish this kind of descriptors from the 
standard descriptors used on IA-64 or HP-PA, so it's contradictory.  Moreover 
I would really start with the "custom" case and not the standard case as was 
originally written, the "custom" case being the common case for targets.

I'm not really convinced by the substitution misalignment -> tag either, but 
if others find the new version more understandable, then OK with me.

On 07/27/2018 03:44 PM, Eric Botcazou wrote:
>> Apropos of the discussion about improving the docs for
>> TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port
>> submission,
>>
>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html
>>
>> here is the patch I've come up with based on reading the source.  Is
>> this technically accurate?  Any suggestions on how to improve it further?
> 
> ""Define this hook to 0 if the target implements custom support"
> 
> "custom" was precisely chosen to distinguish this kind of descriptors from the
> standard descriptors used on IA-64 or HP-PA, so it's contradictory.  Moreover
> I would really start with the "custom" case and not the standard case as was
> originally written, the "custom" case being the common case for targets.

This is precisely what I found so confusing about the original text.  To 
me, "custom" implies that the back end is *customized* to have its own 
descriptor implementation to conform to target-specific ABI standards, 
not that it uses a generic implementation in common code.

> I'm not really convinced by the substitution misalignment -> tag either, but
> if others find the new version more understandable, then OK with me.

I could make the docs say both things, but from the perspective of a 
back end implementor, being explicit that it's a bit mask used to 
differentiate descriptors from any valid function pointer (so you can 
figure out what an appropriate value to define it to is) is more 
critical than describing what target-independent code does with a 
descriptor once it has identified that's what it's got.

-Sandra

> This is precisely what I found so confusing about the original text.  To
> me, "custom" implies that the back end is *customized* to have its own
> descriptor implementation to conform to target-specific ABI standards,
> not that it uses a generic implementation in common code.

To me, "custom" sounds contradictory with "conform to" in this sentence.

And I don't follow your interpretation, back-ends are by definition customized 
to follow external constraints and not every macro/hook has "custom" in it.

> I could make the docs say both things, but from the perspective of a
> back end implementor, being explicit that it's a bit mask used to
> differentiate descriptors from any valid function pointer (so you can
> figure out what an appropriate value to define it to is) is more
> critical than describing what target-independent code does with a
> descriptor once it has identified that's what it's got.

Fair enough.

On 07/27/2018 03:44 PM, Eric Botcazou wrote:
>> Apropos of the discussion about improving the docs for
>> TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port
>> submission,
>>
>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html
>>
>> here is the patch I've come up with based on reading the source.  Is
>> this technically accurate?  Any suggestions on how to improve it further?
> 
> ""Define this hook to 0 if the target implements custom support"
> 
> "custom" was precisely chosen to distinguish this kind of descriptors from the 
> standard descriptors used on IA-64 or HP-PA, so it's contradictory.  Moreover 
> I would really start with the "custom" case and not the standard case as was 
> originally written, the "custom" case being the common case for targets.
> 
> I'm not really convinced by the substitution misalignment -> tag either, but 
> if others find the new version more understandable, then OK with me.
I think the new version is an improvement, but I don't think it's ideal
due to the issues noted above.

We're trying to distinguish between the ABI mandated function
descriptors and those generated internally for supporting nested
functions.  Wordsmithing that into the docs might make things clearer.
I'm not sure how to express that clearly in the name of the target hook
though.

jeff

On 08/03/2018 12:16 PM, Jeff Law wrote:
> On 07/27/2018 03:44 PM, Eric Botcazou wrote:
>>> Apropos of the discussion about improving the docs for
>>> TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port
>>> submission,
>>>
>>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html
>>>
>>> here is the patch I've come up with based on reading the source.  Is
>>> this technically accurate?  Any suggestions on how to improve it further?
>>
>> ""Define this hook to 0 if the target implements custom support"
>>
>> "custom" was precisely chosen to distinguish this kind of descriptors from the
>> standard descriptors used on IA-64 or HP-PA, so it's contradictory.  Moreover
>> I would really start with the "custom" case and not the standard case as was
>> originally written, the "custom" case being the common case for targets.
>>
>> I'm not really convinced by the substitution misalignment -> tag either, but
>> if others find the new version more understandable, then OK with me.
> I think the new version is an improvement, but I don't think it's ideal
> due to the issues noted above.
> 
> We're trying to distinguish between the ABI mandated function
> descriptors and those generated internally for supporting nested
> functions.  Wordsmithing that into the docs might make things clearer.
> I'm not sure how to express that clearly in the name of the target hook
> though.

Hmmm.  I agree that the name of the hook is confusing.  Part of the 
problem is that it does two different things, but I don't have any good 
ideas for how to fix that.

Here's another attempt at documenting what's there now, this time 
avoiding using the ambiguous term "custom" at all in the text.  I've 
also tried to address the other comments.  Is this any better?

-Sandra
2018-08-05  Sandra Loosemore  <sandra@codesourcery.com>

	gcc/
	* target.def (custom_function_descriptors): Improve documentation.
	* doc/tm.texi.in (Trampolines): Expand discussion of function
	descriptors and move TARGET_CUSTOM_FUNCTION_DESCRIPTORS to the
	beginning of the section.
	* doc/tm.texi: Regenerated.
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 263300)
+++ gcc/doc/tm.texi	(working copy)
@@ -5267,24 +5267,78 @@ into the stack.  Arguments meaning is si
 @end deftypefn
 
 @node Trampolines
-@section Trampolines for Nested Functions
+@section Support for Nested Functions
+@cindex support for nested functions
 @cindex trampolines for nested functions
-@cindex nested functions, trampolines for
+@cindex descriptors for nested functions
+@cindex nested functions, support for
 
-A @dfn{trampoline} is a small piece of code that is created at run time
-when the address of a nested function is taken.  It normally resides on
-the stack, in the stack frame of the containing function.  These macros
-tell GCC how to generate code to allocate and initialize a
-trampoline.
-
-The instructions in the trampoline must do two things: load a constant
-address into the static chain register, and jump to the real address of
-the nested function.  On CISC machines such as the m68k, this requires
-two instructions, a move immediate and a jump.  Then the two addresses
-exist in the trampoline as word-long immediate operands.  On RISC
-machines, it is often necessary to load each address into a register in
-two parts.  Then pieces of each address form separate immediate
-operands.
+Taking the address of a nested function requires special compiler
+handling to ensure that the static chain register is loaded when
+the function is invoked via an indirect call.
+
+GCC has traditionally supported nested functions by creating an
+executable @dfn{trampoline} at run time when the address of a nested
+function is taken.  This is a small piece of code which normally
+resides on the stack, in the stack frame of the containing function.
+The trampoline loads the static chain register and then jumps to the
+real address of the nested function.
+
+The use of trampolines requires an executable stack, which is a
+security risk.  To avoid this problem, GCC also supports another
+strategy: using descriptors for nested functions.  Under this model,
+taking the address of a nested function results in a pointer to a
+non-executable function descriptor object.  Initializing the static chain
+from the descriptor is handled at indirect call sites.
+
+On some targets, including HPPA and IA-64, function descriptors may be
+mandated by the ABI or be otherwise handled in a target-specific way
+by the back end in its code generation strategy for indirect calls.
+GCC also provides its own generic descriptor implementation to support the
+@option{-fno-trampolines} option.  In this case runtime detection of
+function descriptors at indirect call sites relies on descriptor
+pointers being tagged with a bit that is never set in bare function
+addresses.  Since GCC's generic function descriptors are
+not ABI-compliant, this option is typically used only on a
+per-language basis (notably by Ada) or when it can otherwise be
+applied to the whole program.
+
+Define the following hook if your backend either implements ABI-specified
+descriptor support, or can use GCC's generic descriptor implementation
+for nested functions.
+
+@deftypevr {Target Hook} int TARGET_CUSTOM_FUNCTION_DESCRIPTORS
+If the target can use GCC's generic descriptor mechanism for nested
+functions, define this hook to a power of 2 representing an unused bit
+in function pointers which can be used to differentiate descriptors at
+run time.  This value gives the number of bytes by which descriptor
+pointers are misaligned compared to function pointers.  For example, on
+targets that require functions to be aligned to a 4-byte boundary, a
+value of either 1 or 2 is appropriate unless the architecture already
+reserves the bit for another purpose, such as on ARM.
+
+Define this hook to 0 if the target implements ABI support for
+function descriptors in its standard calling sequence, like for example
+HPPA or IA-64.
+
+Using descriptors for nested functions
+eliminates the need for trampolines that reside on the stack and require
+it to be made executable.
+@end deftypevr
+
+The following macros tell GCC how to generate code to allocate and
+initialize an executable trampoline.  You can also use this interface
+if your back end needs to create ABI-specified non-executable descriptors; in
+this case the "trampoline" created is the descriptor containing data only.
+
+The instructions in an executable trampoline must do two things: load
+a constant address into the static chain register, and jump to the real
+address of the nested function.  On CISC machines such as the m68k,
+this requires two instructions, a move immediate and a jump.  Then the
+two addresses exist in the trampoline as word-long immediate operands.
+On RISC machines, it is often necessary to load each address into a
+register in two parts.  Then pieces of each address form separate
+immediate operands.
 
 The code generated to initialize the trampoline must store the variable
 parts---the static chain value and the function address---into the
@@ -5351,24 +5405,6 @@ be returned; otherwise @var{addr} should
 If this hook is not defined, @var{addr} will be used for function calls.
 @end deftypefn
 
-@deftypevr {Target Hook} int TARGET_CUSTOM_FUNCTION_DESCRIPTORS
-This hook should be defined to a power of 2 if the target will benefit
-from the use of custom descriptors for nested functions instead of the
-standard trampolines.  Such descriptors are created at run time on the
-stack and made up of data only, but they are non-standard so the generated
-code must be prepared to deal with them.  This hook should be defined to 0
-if the target uses function descriptors for its standard calling sequence,
-like for example HP-PA or IA-64.  Using descriptors for nested functions
-eliminates the need for trampolines that reside on the stack and require
-it to be made executable.
-
-The value of the macro is used to parameterize the run-time identification
-scheme implemented to distinguish descriptors from function addresses: it
-gives the number of bytes by which their address is misaligned compared
-with function addresses.  The value of 1 will generally work, unless it is
-already reserved by the target for another purpose, like for example on ARM.
-@end deftypevr
-
 Implementing trampolines is difficult on many machines because they have
 separate instruction and data caches.  Writing into a stack location
 fails to clear the memory in the instruction cache, so when the program
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	(revision 263300)
+++ gcc/doc/tm.texi.in	(working copy)
@@ -3766,24 +3766,61 @@ These machine description macros help im
 @hook TARGET_SETUP_INCOMING_VARARG_BOUNDS
 
 @node Trampolines
-@section Trampolines for Nested Functions
+@section Support for Nested Functions
+@cindex support for nested functions
 @cindex trampolines for nested functions
-@cindex nested functions, trampolines for
+@cindex descriptors for nested functions
+@cindex nested functions, support for
 
-A @dfn{trampoline} is a small piece of code that is created at run time
-when the address of a nested function is taken.  It normally resides on
-the stack, in the stack frame of the containing function.  These macros
-tell GCC how to generate code to allocate and initialize a
-trampoline.
-
-The instructions in the trampoline must do two things: load a constant
-address into the static chain register, and jump to the real address of
-the nested function.  On CISC machines such as the m68k, this requires
-two instructions, a move immediate and a jump.  Then the two addresses
-exist in the trampoline as word-long immediate operands.  On RISC
-machines, it is often necessary to load each address into a register in
-two parts.  Then pieces of each address form separate immediate
-operands.
+Taking the address of a nested function requires special compiler
+handling to ensure that the static chain register is loaded when
+the function is invoked via an indirect call.
+
+GCC has traditionally supported nested functions by creating an
+executable @dfn{trampoline} at run time when the address of a nested
+function is taken.  This is a small piece of code which normally
+resides on the stack, in the stack frame of the containing function.
+The trampoline loads the static chain register and then jumps to the
+real address of the nested function.
+
+The use of trampolines requires an executable stack, which is a
+security risk.  To avoid this problem, GCC also supports another
+strategy: using descriptors for nested functions.  Under this model,
+taking the address of a nested function results in a pointer to a
+non-executable function descriptor object.  Initializing the static chain
+from the descriptor is handled at indirect call sites.
+
+On some targets, including HPPA and IA-64, function descriptors may be
+mandated by the ABI or be otherwise handled in a target-specific way
+by the back end in its code generation strategy for indirect calls.
+GCC also provides its own generic descriptor implementation to support the
+@option{-fno-trampolines} option.  In this case runtime detection of
+function descriptors at indirect call sites relies on descriptor
+pointers being tagged with a bit that is never set in bare function
+addresses.  Since GCC's generic function descriptors are
+not ABI-compliant, this option is typically used only on a
+per-language basis (notably by Ada) or when it can otherwise be
+applied to the whole program.
+
+Define the following hook if your backend either implements ABI-specified
+descriptor support, or can use GCC's generic descriptor implementation
+for nested functions.
+
+@hook TARGET_CUSTOM_FUNCTION_DESCRIPTORS
+
+The following macros tell GCC how to generate code to allocate and
+initialize an executable trampoline.  You can also use this interface
+if your back end needs to create ABI-specified non-executable descriptors; in
+this case the "trampoline" created is the descriptor containing data only.
+
+The instructions in an executable trampoline must do two things: load
+a constant address into the static chain register, and jump to the real
+address of the nested function.  On CISC machines such as the m68k,
+this requires two instructions, a move immediate and a jump.  Then the
+two addresses exist in the trampoline as word-long immediate operands.
+On RISC machines, it is often necessary to load each address into a
+register in two parts.  Then pieces of each address form separate
+immediate operands.
 
 The code generated to initialize the trampoline must store the variable
 parts---the static chain value and the function address---into the
@@ -3815,8 +3852,6 @@ is used for aligning trampolines.
 
 @hook TARGET_TRAMPOLINE_ADJUST_ADDRESS
 
-@hook TARGET_CUSTOM_FUNCTION_DESCRIPTORS
-
 Implementing trampolines is difficult on many machines because they have
 separate instruction and data caches.  Writing into a stack location
 fails to clear the memory in the instruction cache, so when the program
Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 263300)
+++ gcc/target.def	(working copy)
@@ -4985,21 +4985,22 @@ If this hook is not defined, @var{addr}
 
 DEFHOOKPOD
 (custom_function_descriptors,
- "This hook should be defined to a power of 2 if the target will benefit\n\
-from the use of custom descriptors for nested functions instead of the\n\
-standard trampolines.  Such descriptors are created at run time on the\n\
-stack and made up of data only, but they are non-standard so the generated\n\
-code must be prepared to deal with them.  This hook should be defined to 0\n\
-if the target uses function descriptors for its standard calling sequence,\n\
-like for example HP-PA or IA-64.  Using descriptors for nested functions\n\
-eliminates the need for trampolines that reside on the stack and require\n\
-it to be made executable.\n\
+ "If the target can use GCC's generic descriptor mechanism for nested\n\
+functions, define this hook to a power of 2 representing an unused bit\n\
+in function pointers which can be used to differentiate descriptors at\n\
+run time.  This value gives the number of bytes by which descriptor\n\
+pointers are misaligned compared to function pointers.  For example, on\n\
+targets that require functions to be aligned to a 4-byte boundary, a\n\
+value of either 1 or 2 is appropriate unless the architecture already\n\
+reserves the bit for another purpose, such as on ARM.\n\
+\n\
+Define this hook to 0 if the target implements ABI support for\n\
+function descriptors in its standard calling sequence, like for example\n\
+HPPA or IA-64.\n\
 \n\
-The value of the macro is used to parameterize the run-time identification\n\
-scheme implemented to distinguish descriptors from function addresses: it\n\
-gives the number of bytes by which their address is misaligned compared\n\
-with function addresses.  The value of 1 will generally work, unless it is\n\
-already reserved by the target for another purpose, like for example on ARM.",\
+Using descriptors for nested functions\n\
+eliminates the need for trampolines that reside on the stack and require\n\
+it to be made executable.",\
  int, -1)
 
 /* Return the number of bytes of its own arguments that a function

> Here's another attempt at documenting what's there now, this time
> avoiding using the ambiguous term "custom" at all in the text.  I've
> also tried to address the other comments.  Is this any better?

Yes, it's fine with me, thanks.