From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 34188 invoked by alias); 4 Mar 2020 16:24:45 -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 34112 invoked by uid 89); 4 Mar 2020 16:24:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.1 spammy=warning-free, warningfree 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; Wed, 04 Mar 2020 16:24:36 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 94997ADB3; Wed, 4 Mar 2020 16:24:27 +0000 (UTC) Subject: Re: [PATCH] Replace IgnoreSize/DefaultSize with MnemonicSize To: "H.J. Lu" Cc: Binutils References: <20200303140920.GA329930@gmail.com> <97ab1ae8-ef86-eb15-68c1-a4a09cccd011@suse.com> From: Jan Beulich Message-ID: <127bebb1-0ae7-593e-695b-13c0cb50e9f6@suse.com> Date: Wed, 04 Mar 2020 16:24: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/msg00102.txt On 03.03.2020 20:31, H.J. Lu wrote: > On Tue, Mar 3, 2020 at 9:26 AM Jan Beulich wrote: >> >> 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. > > I appreciate your intention. But the primary goal of gas is to serve GCC. > In this case, there are ino issues with integer conversion in GCC 8 and we > have to support existing GCC 8. Issue a warning in AT&T syntax is not > really an option here. The initial release of gcc 8 was still buggy, and you can then extrapolate this to older versions. If code is to be compiled warning-free with older releases, gcc should get respective backports. Hiding actual bugs (because of gcc shortcomings) is not really an option here (to use your wording). Furthermore - if you were to discover more problems with even older gcc versions, would you then "declare" all those insns as exceptions too? Such an approach doesn't make any sense to me. >>>>> 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 checked in in. Please feel free to replace IgnoreSize on integer conversions > with a new value. As per the series sent earlier today - we're not quite there yet. Jan