From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82f.google.com (mail-qt1-x82f.google.com [IPv6:2607:f8b0:4864:20::82f]) by sourceware.org (Postfix) with ESMTPS id 47580398641D for ; Wed, 27 May 2020 12:43:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 47580398641D Received: by mail-qt1-x82f.google.com with SMTP id x29so5774913qtv.4 for ; Wed, 27 May 2020 05:43:19 -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=SZCwRNFLuRvIXT0Av3D5yNv18ttBVyzGEP3El3l2dJo=; b=IUBdSE8iCIC41gwTIfJjjna8w8fbpJG5Rtxc8QdMs7MutcOOH4hq8LBAqbTyq2XcAR ZUhxYbwYZQ+sO6rp3w96YiznDIHDKKqbLeJsAcU8m/g3iBAR+eDmUNqKIgHYYTGgJBJn CFhQYZsxDu+aM7ISiO3Neb6s8KMQ/1HzEr72azoz7JJc221MOXFnVHAGJjQ+75xL7rvO lgByg63qwlDrZ5eTFGec4OHPDDSHaRdJbMTDbBvtCqEvgNK4zkvTmtfLe/ey9CUqzt6w OCHp/3jPOgUNRvLz+1Qo1askdQJSrQvy7dqzuC46fgUauYaxSK1hrfcM/FfvlCcfC2B4 LqqA== X-Gm-Message-State: AOAM5332wGTKCFsmVMBf8wiyfP0ngPizaINjYVS+EMqp9Ye/G/t8rcFT JPGRJSIfDZNJvtLaVExrzzkJSA== X-Google-Smtp-Source: ABdhPJxoq6T7TcB7vYT5vrjLJFCiqlR0gZWe/zD7v2aTIuiYXBE/G8htimxlm1v9UHYK+LjW5zQdag== X-Received: by 2002:ac8:2679:: with SMTP id v54mr3955706qtv.130.1590583398824; Wed, 27 May 2020 05:43:18 -0700 (PDT) Received: from [192.168.0.185] ([191.34.87.30]) by smtp.gmail.com with ESMTPSA id s15sm2441834qtc.95.2020.05.27.05.43.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 27 May 2020 05:43:17 -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> <31642a68-857c-39b0-35d0-13375fa5810f@linaro.org> <58347066-5A4C-43B2-8F26-2940DCD60F9D@arm.com> From: Luis Machado Message-ID: Date: Wed, 27 May 2020 09:43:13 -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: <58347066-5A4C-43B2-8F26-2940DCD60F9D@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.6 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: Wed, 27 May 2020 12:43:20 -0000 On 5/26/20 1:10 PM, Alan Hayward wrote: > > >> On 26 May 2020, at 13:17, Luis Machado wrote: >> >> 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. > > ...and we don’t want to match those. > > Ok, I’m happy with the patch then :) > > > Alan. > Thanks. Pushed now.