From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id C08E2389853D for ; Tue, 25 May 2021 15:15:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C08E2389853D Received: by mail-ot1-x336.google.com with SMTP id i12-20020a05683033ecb02903346fa0f74dso18264929otu.10 for ; Tue, 25 May 2021 08:15:23 -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=7NU8WhOdePVOSyO+aJeEyGncqImRWLCtlRF/cb2bO5E=; b=ebFKmZHf3BgFHK01ZT+GB0HvANeMvQJwyOMjfJJN4ChkhgOjN9tnJPw1e5Pb9D6RcS NzELDQZOYS2MF7FTXmwsAdeFM5ihGm5bMFcN6SN/fVVrXy33rkuIMg3+HAg83YRjMA/X gOn6s++VvMCgz45zg5r4qJqyDd8b5S6gr32m2tr9+3J6naBHkpvpT6DlpS0sNd4pTHIz SBR+66Ystj0XtiQk4eyZDEE43LAHZdNySoeIFghc2uPQqQPDOhPt3VUGRm1va6SMcytg vWEVHpVQ2mDD0HfAm6T8c+tbb/RqjVOREZDNEiBR44DWP0UtvTHrwVHEUPQ+mwa7eCSO tUWA== X-Gm-Message-State: AOAM533QubALoUnTLF74o7CvU8pGM3Dt5oQIPfzPzeCZqW+3EE9gkEhT Rxc2XZ7iNTB8YUl91lllNcDOu8I60+A= X-Google-Smtp-Source: ABdhPJyOSzkNeEXvfbfhEjPlaRdJME3ZOgN6I3rrDwWTiCgwStQRcNhbuhHzQuTwxazcqlbb+/urHg== X-Received: by 2002:a05:6830:790:: with SMTP id w16mr21477540ots.209.1621955723073; Tue, 25 May 2021 08:15:23 -0700 (PDT) Received: from [192.168.0.41] (174-16-126-108.hlrn.qwest.net. [174.16.126.108]) by smtp.gmail.com with ESMTPSA id x8sm3301949oiv.51.2021.05.25.08.15.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 25 May 2021 08:15:22 -0700 (PDT) Subject: Re: [PATCH 1/2] c-family: Copy DECL_USER_ALIGN even if DECL_ALIGN is similar. To: Robin Dapp , Jason Merrill , GCC Patches References: From: Martin Sebor Message-ID: <9c5e0da6-9e62-b3e8-6d65-5fe2999ced1a@gmail.com> Date: Tue, 25 May 2021 09:15:21 -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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-8.9 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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:15:26 -0000 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! Martin > > 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). > > Regards >  Robin