From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 1C0493858038 for ; Tue, 25 May 2021 15:56:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1C0493858038 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-359-y0xK49vgM66lwER74IQIUw-1; Tue, 25 May 2021 11:56:31 -0400 X-MC-Unique: y0xK49vgM66lwER74IQIUw-1 Received: by mail-qt1-f200.google.com with SMTP id z9-20020a05622a0609b02901f30a4fcf9bso22664021qta.4 for ; Tue, 25 May 2021 08:56:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; 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=et2YXzW6L+1tjBdumTf4xWhpJfzWkZKiK3tlVwXHDN0=; b=SU+I0Lm/CJu+fYAA+nTlaAFk9w4ZH4RTa85xZk+xRMoLbXYOrVYwdOuA4agV7N32/y T/Q4SlXpXJBtDGgKISECsjAkyTXzPf4YEiLFIdR4Q1rXakuBTV+nMh1/GUsAIu69oGZL HFdMWcbTYCiJVwvLfar2jFuBEe7613zlLn2h6YmAqo0M+J78vNwo4W+f4zqfZSx3h+wu eecXfsdL7VcbxmUSBmaMcKzxbTzWv5qdAF0CCjImUgeV/6+odRhsFBBUiyh9MEbeefIK 8y0ypEo6uRsKA2gNf8nMVDkX51ikurThz69tPOUF95O+fSh+3F1V98NTx5N7Tecr1PYf Ec/w== X-Gm-Message-State: AOAM5301vhbd1X7o5dYrIfz7qNeFtrUSZvCXE+YH/N0gt9EV95BoQRJb feeY/uXdmm9zIR1UghBDxJiv9F/kj3Bj5FdMsGPTy3IEARepnOaKVvAos3JbZSNYv2MoxejMB3P k3V7U71nj3+6bnN1E0x3ZLQoe3PxOMzYMzqixl95TBjfPZ62o22n8xfqrz6ZwHfWQsg== X-Received: by 2002:a37:e216:: with SMTP id g22mr35773857qki.254.1621958190054; Tue, 25 May 2021 08:56:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyAvwV8LNtey7PBwPYHMs4CrUCr8SS7niU5kaH1n43x8w9AkSwDXoIAFRccczClLkAD4Q2agg== X-Received: by 2002:a37:e216:: with SMTP id g22mr35773816qki.254.1621958189670; Tue, 25 May 2021 08:56:29 -0700 (PDT) Received: from [192.168.1.148] ([130.44.159.43]) by smtp.gmail.com with ESMTPSA id p63sm13223231qkf.31.2021.05.25.08.56.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 25 May 2021 08:56:29 -0700 (PDT) Subject: Re: [PATCH 1/2] c-family: Copy DECL_USER_ALIGN even if DECL_ALIGN is similar. To: Martin Sebor , Robin Dapp , GCC Patches References: <9c5e0da6-9e62-b3e8-6d65-5fe2999ced1a@gmail.com> From: Jason Merrill Message-ID: Date: Tue, 25 May 2021 11:56:28 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <9c5e0da6-9e62-b3e8-6d65-5fe2999ced1a@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Tue, 25 May 2021 15:56:35 -0000 On 5/25/21 11:15 AM, Martin Sebor wrote: > On 5/25/21 4:38 AM, Robin Dapp wrote: >> Hi Martin and Jason, >> >>>> The removal of the dead code looks good to me.  The change to >>>> "re-init lastalign" doesn't seem right.  When it's zero it means >>>> the conflict is between two attributes on the same declaration, >>>> in which case the note shouldn't be printed (it would just point >>>> to the same location as the warning). >>> >>> Agreed. >> >> Did I get it correctly that you refer to printing a note in e.g. the >> following case? >> >>   inline int __attribute__ ((aligned (16), aligned (4))) >>   finline_align (int); > > Yes, that's what I was referring to. > >> >> I indeed missed this but it could be fixed by checking (on top of the >> patch) >> >> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c >> index 98c98944405..7349da73f14 100644 >> --- a/gcc/c-family/c-attribs.c >> +++ b/gcc/c-family/c-attribs.c >> @@ -2324,7 +2324,7 @@ common_handle_aligned_attribute (tree *node, >> tree name, tree args, int flags, >>         /* Either a prior attribute on the same declaration or one >>           on a prior declaration of the same function specifies >>           stricter alignment than this attribute.  */ >> -      bool note = lastalign != 0; >> +      bool note = last_decl != decl && lastalign != 0; >> >> As there wasn't any FAIL, I would add another test which checks for this. > > That would be great, thank you! > >> I find the whole logic here a bit convoluted but when there is no real >> last_decl, then last_decl = decl.  A note would not be printed before >> the patch because we erroneously warned about the "conflict" of the >> function's default alignment (8) vs the requested alignment (4). Ah, the problem is that we only give this warning because of DECL_USER_ALIGN on last_decl, but then don't use the alignment of last_decl. As you say, the logic is convoluted. Let's simplify it rather than make it more convoluted. One possibility would be to change || to | to avoid the shortcut, and then bool note = lastalign > curalign; if (note) curalign = lastalign; Jason