From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 838553858D28 for ; Mon, 6 Dec 2021 15:25:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 838553858D28 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id B7391212C9; Mon, 6 Dec 2021 15:25:05 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 9697913C52; Mon, 6 Dec 2021 15:25:05 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id KWuqI1ErrmFsVQAAMHmgww (envelope-from ); Mon, 06 Dec 2021 15:25:05 +0000 Subject: Re: [PATCH] [gdb/testsuite] Fix gdb.arch/i386-avx.exp with clang To: Pedro Alves , Andrew Burgess Cc: gdb-patches@sourceware.org References: <20211104135559.5875-1-tdevries@suse.de> <20211105093300.GG918204@redhat.com> <20211105115404.GA1816063@redhat.com> <8074d4d8-fe21-bccf-3fb6-f4be2ea67f7b@palves.net> <1f6ed2db-5d31-9338-226e-3e1a5a7c225b@suse.de> <4a98d0ce-b473-7a44-f399-3a604a5b2516@suse.de> <1104add7-1cb0-efa1-f58a-c2d21846c5dc@palves.net> From: Tom de Vries Message-ID: Date: Mon, 6 Dec 2021 16:25:05 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <1104add7-1cb0-efa1-f58a-c2d21846c5dc@palves.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Dec 2021 15:25:08 -0000 On 11/5/21 2:54 PM, Pedro Alves wrote: > On 2021-11-05 13:35, Tom de Vries wrote: >> On 11/5/21 2:20 PM, Pedro Alves wrote: >>> On 2021-11-05 13:15, Tom de Vries wrote: >>>> On 11/5/21 1:55 PM, Pedro Alves wrote: >>>>> On 2021-11-05 12:23, Tom de Vries via Gdb-patches wrote: >>>>> >>>>>>> No, but in gdb/testsuite/lib/attribute.h we do setup a compatibility >>>>>>> macro for 'noclone', so there's definitely precedent for using >>>>>>> attributes that might not be supported everywhere. >>>>>>> >>>>>> >>>>>> Right, I'm aware of this, but that's a typical case where we have no >>>>>> portable alternative. >>>>> >>>>> We actually do -- _Alignas is standard C11. This fixes the test as well: >>>>> >>>>> _Alignas(32) v8sf_t data[] = >>>>> >>>> >>>> I was referring to the noclone, but ok, I was not aware of the _Alignas, >>>> good to know, thanks. >>>> >>>> Anyway, in the latest version this is not relevant anymore, since the >>>> precise alignment implementation has an extra benefit, as explained in >>>> the post. >>>> >>> >>> OOC, is that benefit important here? >>> >> >> So, this is the post I mentioned ( >> https://sourceware.org/pipermail/gdb-patches/2021-November/183183.html ). >> >> Well, the benefit is that it prevents accidental overalignment, which is >> the reason that this problem escaped detection and/or fixing for so long. >> >> Without that, I could do a thinko and specify too small an alignment and >> have the test passing accidentally, only to fail in a different setup. >> > > The code made no effort at all to align the object, which is I think the main reason > why it went missed. Well, yes, but "no effort at all to align the object" still translates to some minimal required alignment, which was too small, and which was not detected because of accidental over-alignment. So AFAICT, my reasoning is sound here. > As soon as you write some explicit alignment, I don't think > that your proposal helps that much. It helps me by giving me confidence that I hardcoded a large enough alignment. > The new allocator won't help _finding_ the places > that miss alignment directives. Correct, I'm not claiming that. It will help me though in case the instruction requires an alignment of 32 and I misread the documentation and understand and use 16 instead. > I'm honestly not finding the benefit compelling enough to > justify the complication, compared to using alignas/_Alignas, which is what actual user > code will be using. My .2c, anyhow. > Thanks for sharing your opinion on this. I understand what you're saying, but I do think the benefit is compelling enough. I spent a lot of my time trying to reproduce, analyze and fix problems that are only seen on some systems, or intermittently, and consequently I very much value anything that makes test-cases behave the same on different systems. I've now split the patch in two, and committed: - 1 patch implementing the fix with _Alignas https://sourceware.org/pipermail/gdb-patches/2021-December/184249.html - 1 patch following up to use precise align https://sourceware.org/pipermail/gdb-patches/2021-December/184250.html This should help focus the rationale for the second part. It also means that in case of problems, it can be reverted easily without re-introducing the error fixed by the first part. Thanks, - Tom