From patchwork Mon Jan 11 14:37:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrill Tkachov X-Patchwork-Id: 565920 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 149901402D9 for ; Tue, 12 Jan 2016 01:37:17 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=sKi/GeR7; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; q=dns; s= default; b=ua+1OF8Tn43/X/wfWUGMS40gQhWwvtib42ZPyz9fFyhoD7e+DVsDf 5OpIwbBpCG/ePj3HsKqXiIl46PWLgrjknjfsZJwLhrI00DKDhxT9chSvGdKcV+eG 2eyyLFuBgZmGqVMFsFVQrTPqRtlEAWlbdp6BmmqhHizPmviahi+Xpo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; s=default; bh=QNcJkDFQCaq5NcQ0vP+HyqiAZzY=; b=sKi/GeR7Cyak3gVOoX9INCpE9QUQ 72tPNbf5EjAhCEKLAbP1uUHMUlUswb0/U0Nu4qEEG+HpzxS+o0T6BGG0+4U7PlqC bf5zRtObLZb21pARmXcUEwgfSVlsUo0eTd5xAw5YJSXzUJ5tfTlNr8qDx+y1QCc5 t6BvbXvhnpZrvnM= Received: (qmail 92252 invoked by alias); 11 Jan 2016 14:37:10 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 92140 invoked by uid 89); 11 Jan 2016 14:37:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.3 required=5.0 tests=AWL, BAYES_50, KAM_ASCII_DIVIDERS, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=Ask, armcc, fox, arm-c.c X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 11 Jan 2016 14:37:08 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E5A2B4F0; Mon, 11 Jan 2016 06:36:31 -0800 (PST) Received: from [10.2.206.200] (e100706-lin.cambridge.arm.com [10.2.206.200]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 27A513F308; Mon, 11 Jan 2016 06:37:06 -0800 (PST) Message-ID: <5693BE10.9050708@foss.arm.com> Date: Mon, 11 Jan 2016 14:37:04 +0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Christian Bruel CC: "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros References: <568E8708.4050602@st.com> <569392DE.1030809@foss.arm.com> <5693A602.2090207@st.com> In-Reply-To: <5693A602.2090207@st.com> On 11/01/16 12:54, Christian Bruel wrote: > Hi Kyrill, > > On 01/11/2016 12:32 PM, Kyrill Tkachov wrote: >> Hi Christian, >> >> On 07/01/16 15:40, Christian Bruel wrote: >>> as discussed with Kyrill (https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00307.html), this patch avoids confusing (for the testsuite) macro redefinition warning or pedantic errors when the user changes FP versions implicitly with a #pragma >>> GCC target. The warning is kept when the macro is redefined explicitly by the user. >>> >>> tested on arm-linux-gnueabi for {,-mfpu=neon-fp-armv8,-mfpu=neon} >>> >>> >>> Index: config/arm/arm-c.c >>> =================================================================== >>> --- config/arm/arm-c.c (revision 232101) >>> +++ config/arm/arm-c.c (working copy) >>> @@ -23,6 +23,7 @@ >>> #include "c-family/c-common.h" >>> #include "tm_p.h" >>> #include "c-family/c-pragma.h" >>> +#include "stringpool.h" >>> >>> /* Output C specific EABI object attributes. These can not be done in >>> arm.c because they require information from the C frontend. */ >>> @@ -245,8 +246,18 @@ arm_pragma_target_parse (tree args, tree >>> >>> /* Update macros. */ >>> gcc_assert (cur_opt->x_target_flags == target_flags); >>> - /* This one can be redefined by the pragma without warning. */ >>> - cpp_undef (parse_in, "__ARM_FP"); >>> + >>> + /* Don't warn for macros that have context sensitive values depending on >>> + other attributes. >>> + See warn_of_redefinition, Reset after cpp_create_definition. */ >>> + tree acond_macro = get_identifier ("__ARM_NEON_FP"); >>> + C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL ; >>> + >>> + acond_macro = get_identifier ("__ARM_FP"); >>> + C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL; >>> + >>> + acond_macro = get_identifier ("__ARM_FEATURE_LDREX"); >>> + C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL; >> I see this mechanism also being used by rs6000, s390 and spu but I'm not very familiar with it. >> Could you please provide a short explanatino of what NODE_CONDITIONAL means? >> I suspec this is ok, but I'd like to get a better understanding of what's going on here. > > This is part of a larger support for context-sensitive keywords implemented for rs6000 (patch digging https://gcc.gnu.org/ml/gcc-patches/2007-12/msg00306.html). > > On ARM those preprocessor macros are always defined so we don't need to define the macro_to_expand cpp hook. However their value does legitimately change in the specific #pragma target path so we reuse this logic for this path. > The macro will always be correctly recognized on the other paths(#ifdef,...) because the NODE_CONDITIONAL bit is cleared when defined (see cpp_create_definition). The idea of the original rs6000 patch is that if a macro is user-defined it > is not context-sensitive. > So this is absolutely a reuse of a subpart of a larger support, but this logic fits and works well for our goal, given that the preprocessor value can change between target contexts, and that the bit is not set for "normal" builtin > definition. > > In short: Ask `warn_of_redefinition` to be permissive about those macro redefinitions when we come from a pragma target definition, as if we were redefining a context-sensitive macro, the difference is that it is always defined. > > does this sound clear :-) ? > Thanks, it's much clearer now. A couple of comments on the patch then + tree acond_macro = get_identifier ("__ARM_NEON_FP"); + C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL ; So what happens if __ARM_FP was never defined, does get_identifier return NULL_TREE? If so, won't C_CPP_HASHNODE (acond_macro)->flags ICE? Index: testsuite/gcc.target/arm/pr69180.c =================================================================== --- testsuite/gcc.target/arm/pr69180.c (revision 0) +++ testsuite/gcc.target/arm/pr69180.c (working copy) @@ -0,0 +1,16 @@ +/* PR target/69180 + Check that __ARM_NEON_FP redefinition warns for user setting and not for + #pragma GCC target. */ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_neon_ok } */ +/* { dg-options "-mfloat-abi=softfp -mfpu=neon" } */ + I believe we should use /* { dg-add-options arm_neon } */ here. Thanks, Kyrill