From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 127433 invoked by alias); 3 Mar 2020 17:26:35 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 127425 invoked by uid 89); 3 Mar 2020 17:26:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.1 spammy=consensus, H*f:CAMe9rOrDO, H*i:BfCeamns8i2, H*i:CAMe9rOrDO X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 03 Mar 2020 17:26:33 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C8F3EB028; Tue, 3 Mar 2020 17:26:30 +0000 (UTC) Subject: Re: [PATCH] Replace IgnoreSize/DefaultSize with MnemonicSize To: "H.J. Lu" Cc: Binutils References: <20200303140920.GA329930@gmail.com> From: Jan Beulich Message-ID: <97ab1ae8-ef86-eb15-68c1-a4a09cccd011@suse.com> Date: Tue, 03 Mar 2020 17:26:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-03/txt/msg00050.txt On 03.03.2020 18:15, H.J. Lu wrote: > On Tue, Mar 3, 2020 at 6:50 AM Jan Beulich wrote: >> >> On 03.03.2020 15:09, H.J. Lu wrote: >>> I am testing this patch with GCC 8. I will check it in if it fixes >>> regressions in GCC 8 testsuits: >>> >>> https://gcc.gnu.org/ml/gcc-regression/2020-03/msg00008.html >>> >>> H.J. >>> --- >>> According to gas manual, suffix in instruction mnemonics isn't always >>> required: >>> >>> When there is no sizing suffix and no (suitable) register operands to >>> deduce the size of memory operands, with a few exceptions and where long >>> operand size is possible in the first place, operand size will default >>> to long in 32- and 64-bit modes. >> >> Nothing there says that this defaulting is to happen silently. Yet >> _that's_ what my earlier changes altered. The defaulting is still >> the same. And no - SUCH CASES SHOULD NOT GO SILENTLY, neither here >> nor in the MOVSX/MOVZX case. Ambiguities should _always_ be >> pointed out by the assembler. (There may be [and there is] a mode >> in which this goes silently, to be enabled at the programmer's >> risk.) > > It is not going to happen in AT&T syntax. Gas has to support older GCC > without any warnings. Why? What's wrong with pointing out issues even with compiler generated code? In fact iirc gcc used to be buggy in regard of these conversion instructions, and the assembler change helped spot this. >>> This includes cvtsi2sd, cvtsi2ss, vcvtsi2sd, vcvtsi2ss, vcvtusi2sd and >>> vcvtusi2ss. Since they are used in GCC 8 and older GCC releases, they >>> must be allowed without suffix in AT&T syntax. >>> >>> gas/ >>> >>> PR gas/25622 >>> * testsuite/gas/i386/i386.exp: Run x86-64-default-suffix and >>> x86-64-default-suffix-avx. >>> * testsuite/gas/i386/noreg64.s: Remove cvtsi2sd, cvtsi2ss, >>> vcvtsi2sd, vcvtsi2ss, vcvtusi2sd and vcvtusi2ss entries. >>> * testsuite/gas/i386/noreg64.d: Updated. >>> * testsuite/gas/i386/noreg64.l: Likewise. >>> * testsuite/gas/i386/x86-64-default-suffix-avx.d: New file. >>> * testsuite/gas/i386/x86-64-default-suffix.d: Likewise. >>> * testsuite/gas/i386/x86-64-default-suffix.s: Likewise. >>> >>> opcodes/ >>> >>> PR gas/25622 >>> * i386-opc.tbl: Add IgnoreSize to cvtsi2sd, cvtsi2ss, vcvtsi2sd, >>> vcvtsi2ss, vcvtusi2sd and vcvtusi2ss for AT&T syntax. >> >> Oh no. I'm trying to clean up the IgnoreSize mess and you want to >> add new instances for no good reason (yes, there are cases where >> this is actually missing; hopefully I'll get to send out the >> series later this week). > > Since an instruction template can't have both IgnoreSize and DefaultSize, > I am testing this patch and will check it if there are no regressions. Then > we can add one value to MnemonicSize. It seems pretty unrelated here, but is a good change to make, I think. >> I know I can't prevent this going in, but I'm heavily opposed. >> You don't "fix" anything here, you break things. > > I disagree. It's pretty sad that in binutils consensus isn't required for changes to go in. I'll enter a bug in due course in any event. Jan