On 11/22/18 6:09 AM, Rainer Orth wrote:
Hi Christophe,

On Sat, 10 Nov 2018 at 00:43, Martin Sebor <mse...@gmail.com> wrote:

On 11/09/2018 12:59 PM, Jeff Law wrote:
On 10/31/18 10:27 AM, Martin Sebor wrote:
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01473.html

With the C++ bits approved I'm still looking for a review/approval
of the remaining changes: the C front end and the shared c-family
handling of the new built-in.
I thought I acked those with just a couple minor doc nits:

I don't see a formal approval for the rest in my Inbox or in
the archive.

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 8ffb0cd..dcf4747 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2649,8 +2649,9 @@ explicit @code{externally_visible} attributes
are still necessary.
  @cindex @code{flatten} function attribute
  Generally, inlining into a function is limited.  For a function
marked with
  this attribute, every call inside this function is inlined, if possible.
-Whether the function itself is considered for inlining depends on its
size and
-the current inlining parameters.
+Functions declared with attribute @code{noinline} and similar are not
+inlined.  Whether the function itself is considered for inlining depends
+on its size and the current inlining parameters.
Guessing this was from another doc patch that I think has already been
approved

Yes.  It shouldn't be in the latest patch at the link above.

@@ -11726,6 +11728,33 @@ check its compatibility with @var{size}.

  @end deftypefn

+@deftypefn {Built-in Function} bool __builtin_has_attribute
(@var{type-or-expression}, @var{attribute})
+The @code{__builtin_has_attribute} function evaluates to an integer
constant
+expression equal to @code{true} if the symbol or type referenced by
+the @var{type-or-expression} argument has been declared with
+the @var{attribute} referenced by the second argument.  Neither argument
+is valuated.  The @var{type-or-expression} argument is subject to the
same
s/valuated/evaluated/ ?

This should also be fixed in the latest patch at the link above.

Did the implementation change significantly requiring another review
iteration?

I don't think it changed too significantly between the last two
revisions but I don't have a record of anyone having approved
the C FE and the middle-end bits.  (Sorry if I missed it.) Other
than this response from you all I see in the archive is this:

    https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00606.html

Please let me if the last revision is okay to commit.


Hi,

It seems you committed this yesterday as r266335, and I have noticed
new failures:
on both aarch64 and arm:
FAIL: c-c++-common/builtin-has-attribute-3.c  -Wc++-compat  (test for
excess errors)

gcc.log says:
Excess errors:
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:11:25: error:
alignment for 'faligned_1' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:12:25: error:
alignment for 'faligned_2' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:39:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:40:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:47:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:48:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:50:3: error:
size of array 'Assert' is negative
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:52:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:52:3: error:
size of array 'Assert' is negative
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:53:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:56:3: error:
size of array 'Assert' is negative
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:58:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:59:3: error:
alignment for '__builtin_has_attribute_tmp.' must be at least 4
/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:59:3: error:
size of array 'Assert' is negative

on arm only:
gcc.dg/builtin-has-attribute.c (test for excess errors)
gdb.log says:
Excess errors:
/gcc/testsuite/gcc.dg/builtin-has-attribute.c:12:15: error: size of
array 'Assert' is negative

I'm seeing the same on sparc-sun-solaris2.11, plus

+FAIL: c-c++-common/builtin-has-attribute-3.c  -std=gnu++14 (test for excess 
errors)
+FAIL: c-c++-common/builtin-has-attribute-3.c  -std=gnu++17 (test for excess 
errors)
+FAIL: c-c++-common/builtin-has-attribute-3.c  -std=gnu++98 (test for excess 
errors)

I see these errors with my sparc-sun-solaris2.11 cross-compiler:

/ssd/src/gcc/svn/gcc/testsuite/c-c++-common/builtin-has-attribute-3.c:197:3: error: constructor priorities are not supported

Apparently the sparc-sun-solaris2.11 target doesn't like ctor and
dtor priorities.  The error comes from here:

static priority_type
get_priority (tree args, bool is_destructor)
{
  HOST_WIDE_INT pri;
  tree arg;

  if (!args)
    return DEFAULT_INIT_PRIORITY;

  if (!SUPPORTS_INIT_PRIORITY)
    {
      if (is_destructor)
        error ("destructor priorities are not supported");
      else
        error ("constructor priorities are not supported");
      return DEFAULT_INIT_PRIORITY;
    }

The manual doesn't mention anything about this.  It makes it sound
like ctor/dtor priorities are supported everywhere.

Does Solaris really not support these even with Binutils?  If not,
I'll adjust the test (and the manual to make it clear the attribute
isn't supported everywhere).

According to gcc-testresults postings, several other targets are also
affected:

        aarch64-unknown-linux-gnu
         armv8l-unknown-linux-gnueabihf
         m68k-unknown-linux-gnu
         mips64el-unknown-linux-gnu
         moxie-unknown-elf
         powerpc64-unknown-linux-gnu
         powerpc64le-unknown-linux-gnu

AFAICS, powerpc64le-unknown-linux-gnu does accept priorities so
the test failure there must be something due to something else.
I haven't checked what yet.

Martin

Reply via email to