From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x833.google.com (mail-qt1-x833.google.com [IPv6:2607:f8b0:4864:20::833]) by sourceware.org (Postfix) with ESMTPS id 22A05385DC00 for ; Tue, 26 May 2020 12:17:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 22A05385DC00 Received: by mail-qt1-x833.google.com with SMTP id x12so15845185qts.9 for ; Tue, 26 May 2020 05:17:39 -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:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=ZpC9dMY4FG2uI4qFU9Cru/uwiWyXtvdm3BDiuL5E/pY=; b=e2zEFiNcgqd0qvYuI66E2hfjZtILXqfs2b5l0uCRx/1ufKtDq+qaIzMSxQUC0n5Luc GiBl738KQNvmeEKWRpF2pkN7R8Csw967clXvt3qlmWU4vm10ljhnLLKOSrpZeiY0fKVg jjZvFSnv1HvA8FNKamCJNetvLOBSV+6A47DUKF2mzuIy6gRBmbpgTZKCnrQB/FabXO9Z StmfGf98rnSK8Z99dIwWDdOshCW7M9UWB78Wltz+xsUeCZQvAjEG0KWp6jM6PZZnKiz7 1kSAPaduTOtL06E3IfwwiTpsNFKkL1JEARCPBM6lZE2k6CkJA7FwsnRjLvSc0Y9UpTnt sSMw== X-Gm-Message-State: AOAM531vJlaVGvyVIuc2F7LmUBckB8pk9VTr92COE9PqLVeuJN/jA0ly hNbH8G8h5f6GxpzuIiFI3+QVbg== X-Google-Smtp-Source: ABdhPJyzoT6iK8WlWE1T23hwVf1gJsQK8ZIOhtXk5lKtXeVuLBKPvaLVBbuFlj+H4Mqk7/5w/octFw== X-Received: by 2002:aed:2412:: with SMTP id r18mr933823qtc.12.1590495458524; Tue, 26 May 2020 05:17:38 -0700 (PDT) Received: from [192.168.0.185] ([191.34.87.30]) by smtp.gmail.com with ESMTPSA id o13sm13452437qtk.73.2020.05.26.05.17.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 26 May 2020 05:17:37 -0700 (PDT) Subject: Re: -Wtautological-bitwise-compare error in arm-tdep.c To: Alan Hayward Cc: Simon Marchi , "gdb-patches@sourceware.org" , nd References: <98a9d90b-0452-5b53-b707-9441ebcad6b7@linaro.org> <91440d8b-14c9-611f-ebfd-4cc209c05af8@polymtl.ca> From: Luis Machado Message-ID: <31642a68-857c-39b0-35d0-13375fa5810f@linaro.org> Date: Tue, 26 May 2020 09:17:33 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 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=-3.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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: 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: Tue, 26 May 2020 12:17:40 -0000 On 5/26/20 6:45 AM, Alan Hayward wrote: > > >> On 25 May 2020, at 14:51, Luis Machado wrote: >> >> On 5/25/20 10:49 AM, Simon Marchi wrote: >>> On 2020-05-25 9:08 a.m., Luis Machado wrote: >>>> This fixes an instruction mask typo. We should be matching only >>>> ldrd (immediate) and not any other of its variants. As is, it never matches >>>> anything. >>> And moreover, within the `ldrd (immediate)` instruction, it only matches the >>> `Offset variant` variant, right? >> >> That's right. We don't want to handle anything that changes the SP here. And the post-indexed and pre-indexed variants do so. >> >>>> >>>> With the patch, the instruction mask also allows matching of ldrd (literal), >>>> but the check for SP discards this particular instruction pattern, as it has >>>> a hardcoded PC register. >>> I don't feel the most qualified to approve this patch. Alan, could you please >>> take a look? >>> Simon > > > The maths looks good now. > > However, Binutils uses a slightly different mask, 0xff50: > > > {ARM_FEATURE_CORE_LOW (ARM_EXT_V6T2), > 0xe9500000, 0xff500000, > "ldrd%c\t%12-15r, %8-11r, [%16-19r, #%23`-%0-7W]%21'!%L”}, > > > It does use 0xff70 for a different variation of ldrd: > > {ARM_FEATURE_CORE_LOW (ARM_EXT_V6T2), > 0xe8600000, 0xff700000, > "strd%c\t%12-15r, %8-11r, [%16-19r], #%23`-%0-7W%L"}, > {ARM_FEATURE_CORE_LOW (ARM_EXT_V6T2), > 0xe8700000, 0xff700000, > "ldrd%c\t%12-15r, %8-11r, [%16-19r], #%23`-%0-7W%L”}, > > > That’s in binutils-gdb/opcodes/arm-dis.c. > All that code was added at the same time in 2015. > > 0xff50 is going to allow more matches than 0xff70. > And given that the thing we care about is matching the opcode, > then 0xff50 is safer. > > Before I go off and start looking at instruction decodings, > Luis - where did you get 0xff70 from? From the manual, a concatenation of 7 bits from the opcode plus 4 bits of op0 and 1 bit for Load/Store. Mask 0xfe00 filters the opcode (1110100). Mask 0x160 filters op0 (bits 0, 1 and 3 - 1011). Mask 0x10 filters the load/store bit (load is 1). We want to match the insn pattern 0xe950, which is ldrd (immediate). Compared to 0xff70, with mask 0xff50 we would ignore op<0>. Ignoring op<0> would allow matching ldrd (immediate, pre-indexed) as well.