From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id 18B433857403 for ; Thu, 28 Oct 2021 16:28:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 18B433857403 Received: by mail-pf1-x429.google.com with SMTP id a26so6455196pfr.11 for ; Thu, 28 Oct 2021 09:28:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=7hB5S7VtMfxMRYiTCgfb/BXt7fUEfZiyqdoXgWeEo7A=; b=0Oo6DkIcd9jOWnzyndVIUWFT8sXAtEP/2NPdwCGzm4Wp4ETVHd7n8iBNdeI6pHAUKi a8T6FbY2bsCV5CjO2Y0jZ8OxsoiTW0rZII6Z7GpLDTPNtpoDc3wmQTq/pKow+eC0L3N1 ibCeAgNXVvJmMcNNU3oqb1uu411c/UDM9GTeAS5zPm7tdkimg2aZYW8RrPmt9Nxk+VjU w8uW7a02rwhZiHnLzKHNX1uiwcsqmu1qNazCwKotU1ZmbUxJ1UiBN3jgzXj9hC+OhrNN kQzWoDD8hhRmYjS1Xcm/Q+Is01SLTY8agQNYI619WA9AK+sWU9E84Dpl9xgNdGXfOniT qWeg== X-Gm-Message-State: AOAM5337+I8XyIt/cDHcHp4CuKQE2f0tE4GvAhQO1tc8hGuihdZ9a208 2QHJDoMyqXmsyd+UJwmQtkU= X-Google-Smtp-Source: ABdhPJz7MHcOP54nm1KkaO/fSG/7noiN1HO7zqhkWV/KkducMO3o+dnQX+yO34oDZZ6siQhxY8d7mA== X-Received: by 2002:a63:ac57:: with SMTP id z23mr3883623pgn.164.1635438528050; Thu, 28 Oct 2021 09:28:48 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-116.hlrn.qwest.net. [184.96.250.116]) by smtp.gmail.com with ESMTPSA id c21sm4365523pfl.15.2021.10.28.09.28.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 28 Oct 2021 09:28:47 -0700 (PDT) Subject: Re: [Patch] libcpp: Fix _Pragma expansion [PR102409] To: Tobias Burnus , gcc-patches , Jakub Jelinek , Joseph Myers , David Malcolm References: <012cb5da-5255-ac5e-c7dc-f7eb2bd34955@codesourcery.com> From: Martin Sebor Message-ID: Date: Thu, 28 Oct 2021 10:28:45 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <012cb5da-5255-ac5e-c7dc-f7eb2bd34955@codesourcery.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Oct 2021 16:28:50 -0000 On 10/28/21 9:51 AM, Tobias Burnus wrote: > Before this patch, running > > #define TEST(T) T > #define PARALLEL(X) TEST(X) > PARALLEL( >     for (int i = 0; i < N; i++) { \ >       _Pragma("omp ordered") \ >       S[0] += C[i] + D[i]; \ >     }) > > through 'gcc -E' yielded > > #pragma omp ordered >  for (int i = 0; i < N; i++) { S[0] += C[i] + D[i]; } > > Note that the '#pragma omp ordered' is now above the loop, i.e. before > all macro arguments (and macro expansions). > > With the patch, the result is the following, which matches Clang and I > assume GCC before 4.2 or 4.3, but I have no GCC 4.x available: > > for (int i = 0; i < N; i++) { > #pragma omp ordered >  S[0] += C[i] + D[i]; } There are a number of bug reports of _Pragma not working right in macros, including (and especially) to control diagnostics: https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=_Pragma%20macro&list_id=328003 Just by the description this change seems like it could also fix some of them. It would be helpful to check to see if it does and if so, add tests and resolve the bugs it fixes (I'm willing to help with that in stage 3). Martin > > > The reason seems to be the addition done for PR34692 in r131819, which > added code to avoid an ICE with > FOO( > #pragma GCC diagnostic > ) > > There is a length description in macro.c about what it does and the > pragma_buff which is passed around, including to the now modified > collect_args. Namely, the comment above enter_macro_context states: > >    If there were additionally any unexpanded deferred #pragma >    directives among macro arguments, push another context containing >    the pragma tokens before the yet-to-be-rescanned replacement list >    and return two. > > While that seems to work fine with #pragma, it obviously does not do > what it should for _Pragma. The solution in the patch was to add a > flag to distinguish the CPP_PRAGMA coming from the _Pragma operator > (alias BT_PRAGMA) from the CPP_PRAGMA coming from a user's #pragma. > > OK for mainline? – It is a long-standing regression, but it hasn't > been reported for a while. Thus: how do you feel about backporting? > > I did test it with a full bootstrap + regtesting. I also tested > omptests (cf. PR). > > Tobias > > PS: I had the hope that it would fix some of the other _Pragma related > PRs (see e.g. refs in this PR102409 or search Bugzilla), but it does > not seem to help for those. I do note that most of them are related to > diagnostic. In particular, for PR91669, the output of gcc -E is the > same for GCC 10, for a patched GCC and for clang-11, which makes the > result (issue unaffected by this patch) not that surprising. > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, > 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: > Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; > Registergericht München, HRB 106955