Re: [PATCH v2][C][ADA] use function descriptors instead of trampolines in C

[Joseph Myers <joseph at codesourcery dot com>]

On Mon, 20 Aug 2018, Uecker, Martin wrote:

> This is a new version which adds proper changelog entries and
> a test case (no actual code changes).

Please include the overall description of a change in every version 
submitted.  That is, the patch submission message should both include a 
description of the current version (as in a git-style commit message) and, 
if relevant, a description of what changed relative to the previous 
version of the patch (which would not go in the commit message).

A key thing I'm not clear on is what the user-visible difference in 
compiler behavior is supposed to be with this patch.  Whatever that 
user-visible difference is, I'd expect it to result in some change to the 
documentation of -ftrampolines in invoke.texi (describing the new feature, 
or changing a description of a limitation of an existing feature, or 
something like that).

> +/* { dg-do run { target x86_64-*-* } } */

It is always wrong for a test to use x86_64-*-* like that, because 
anything that should be tested for 64-bit code generation for an x86_64 
target should also be tested for i[34567]86-*-* -m64, and if you don't 
want to test for 32-bit code generation, you need to avoid testing for 
x86_64-*-* -m32, which that test would test for.  Anything genuinely 
x86-specific should go in gcc.target/i386 and then be conditioned on 
effective-target keywords such as lp64 if necessary.

I don't see why this is target-specific (if it is, the documentation for 
users in invoke.texi should explain what targets it works for and what it 
doesn't work for) anyway.  I'd expect it to be a target-independent 
feature with a target-independent test or tests.

Once there is sufficient user-level documentation showing what the 
intended semantics are, then it may be possible to evaluate how the 
implementation achieves that.

-- 
Joseph S. Myers
joseph@codesourcery.com