From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107143 invoked by alias); 22 Mar 2018 13:36:29 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 107067 invoked by uid 89); 22 Mar 2018 13:36:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=doubts X-HELO: EUR02-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr20056.outbound.protection.outlook.com (HELO EUR02-VE1-obe.outbound.protection.outlook.com) (40.107.2.56) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 22 Mar 2018 13:36:27 +0000 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Sudi.Das@arm.com; Received: from [10.2.206.246] (217.140.96.140) by AM3PR08MB0215.eurprd08.prod.outlook.com (2a01:111:e400:8853::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.588.14; Thu, 22 Mar 2018 13:36:23 +0000 Subject: Re: [Aarch64] Fix conditional branches with target far away. To: Sameera Deshpande , Ramana Radhakrishnan Cc: James Greenhalgh , Marcus Shawcroft , Richard Earnshaw , Richard Sandiford , gcc-patches , nd References: <318513c3-9bee-6ef0-11cc-c7c43e5144a2@arm.com> From: Sudakshina Das Message-ID: Date: Thu, 22 Mar 2018 13:42:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: AM5P190CA0011.EURP190.PROD.OUTLOOK.COM (2603:10a6:206:14::24) To AM3PR08MB0215.eurprd08.prod.outlook.com (2a01:111:e400:8853::25) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: a042282a-a556-4077-968d-08d58ff9e79b X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(2017052603328)(7153060)(7193020);SRVR:AM3PR08MB0215; X-Microsoft-Exchange-Diagnostics: 1;AM3PR08MB0215;3:PIL560iXoo4oij25F7MOLFEKHwNrUzJignyjX/KDjjTFTH4ntvD5BBRRzt/Y+fl0o6SII3xiZ1CQHf/PXrPW5j7Dtnw/hqJdytvASZMXobXRHXu3TM+tRvmCavQkKrYEGN/Qf9caRfcnhv5DMXYynAlRSeywP1Ya6iVG+m1fT7T1HvQAXUwKVSswH5ZkbWc3Xfb4ocvzPzzsowL/lML6CHlw+FtpI9AvuTxnXru8HvpffpCVGD0egECNOsB7+pPs;25:E8VxPySquFAgFYps6RCOcaIZPp/4Z+GYv4kwuwHsKDaTQIP8nsUadFLKy2T5UrFyDw4NksbPRyO4AFGeQcoeRn77IGJghFehAsXwNsJBSJfASE9z59gC/VqcUZ+fmP0yG9zDKEGOD/WZXMk1F+mS9dXI3IkNpyqoa1G1KSFGVWSpCm4QGP7KkXJW6EM4bG5rsmq0NCONZnhjYGmWS0SrIrw/CXVB7YPPpePfeHR/GGWM+F9416VImlBrBcC+GsOeC5EXvJS9kWoHR+8Fq+ieLLM1x3mIw2vptU/dOZ3kPYEcJuWgfn+fexsXLXzow/QJ9CmEFidZj8tt6mVLZ7f8Ew==;31:uQK4PtbrghBeTx+XQO3ktaaFNxnCTFtTUAPTzs8vW9Phh3gx8OD6eaiYGK9LmgghUeXjiJ0Q9SCk1diFWmAkKxXXG0D/TddGYpys10fyai6u04UImck9SLXtOXo0nqMjDUK0O9GALU2IBdU7CQYadSDaDteLa7G1fEgEI4WwvQEaWOf+lu2r8GEoMSdl87hPx/GgTxOKbm4GqGDfSm5NYqSllKRocpOfRYNBRBHU3rw= X-MS-TrafficTypeDiagnostic: AM3PR08MB0215: NoDisclaimer: True X-Microsoft-Exchange-Diagnostics: 1;AM3PR08MB0215;20:sQErX6/tQZlZECjoZd0gB7R8c9HYasDx2qFHdXOeE0K+uiXixWSjQFYJXG7Q1htI0Nf2sEUZgD8XXVKochPngVXAzue0gnnQxTIceye74vQp4XAsO4Ub3CHfcpIGyNQbrBzul1pU9Y9qYVDafmMngXJGEnSiK4zYXDsnbyaoKpI=;4:6YYJyAfhYTUlQzS4vy3XaJcs72voqQu26ADbA/GaxZMOL/H5ofrlXbuHhXfHh4C565oEpsDLV57YIt7a3x3l01ssEjP9W7QXZ30znUj0m+mITKytoaW6WIx/OB/jEd6vlAm5dat5EnY1qIX/tFLN8iUs+Lp/zPGWcAoPG47Qx2PUO/ha1eiS/JNBpVZhHwLdDdp+0BuXMyBzDmjqcO5S8n3RmpAgN5e6ZvScqUT403Vn+u8HOs7mwn+HZegdcwQ6cbvPnmB3gB98sKLGwYofK6P/fBXuix5NGWxrvNXbrKP/gvM7Yu8BGVuHtfSRSzVcV4hfysbYytbjrs5JnYCRL4nHcmmKn35qismBJPbQA3M= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(180628864354917)(8415204561270); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231221)(944501327)(52105095)(93006095)(93001095)(3002001)(10201501046)(6055026)(6041310)(20161123558120)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(20161123562045)(6072148)(201708071742011);SRVR:AM3PR08MB0215;BCL:0;PCL:0;RULEID:;SRVR:AM3PR08MB0215; X-Forefront-PRVS: 0619D53754 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6049001)(366004)(39380400002)(396003)(376002)(346002)(39860400002)(189003)(199004)(377424004)(54534003)(5660300001)(305945005)(106356001)(54906003)(478600001)(2906002)(110136005)(72206003)(446003)(93886005)(3846002)(6116002)(7736002)(230700001)(66066001)(316002)(65806001)(86362001)(36756003)(47776003)(229853002)(64126003)(25786009)(16526019)(16576012)(6246003)(31686004)(65956001)(52146003)(2486003)(65826007)(97736004)(81166006)(67846002)(68736007)(386003)(58126008)(31696002)(23676004)(53546011)(5890100001)(53936002)(8676002)(77096007)(6486002)(2950100002)(52116002)(26005)(81156014)(50466002)(76176011)(105586002)(8936002)(4326008)(299355004);DIR:OUT;SFP:1101;SCL:1;SRVR:AM3PR08MB0215;H:[10.2.206.246];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtBTTNQUjA4TUIwMjE1OzIzOmN2TURSSVlSem1tNEFqZ2plTDRTeVdOYTJ2?= =?utf-8?B?Y2F3b1IzUi94clVNc0tJY1d5RkpUWnRHc3NWUldjVlVkaEcyOXJ1azM1aVI1?= =?utf-8?B?N2cweXI2Ky93dWowaHN1dndwNVZ5bmpqREVKU3lMd0RwNVdFZDN1ZVBSSGNm?= =?utf-8?B?U3RwTENTZXUzYVk0ZkM5Z08zcUlpUUZUaGlyMCtkM2FFcGhJTjJXMXhpSnpZ?= =?utf-8?B?T2pzVEdsM3pmQWIxb1VBZExIaStnMXdEcmliQlJ4S3E4anpCN2hsV2k2WjhZ?= =?utf-8?B?ek93Z3J0K1FsN2YvWFhiY29nMEc4YjlkN2NWTmVQZEFBcll4RXJQVjB3TjBN?= =?utf-8?B?enprWUtrWjdVV3JkQjdTMWNSd21XQVYvemNFRzgwdFVYWjloNXVicE51V0RP?= =?utf-8?B?QzhuNkZsUlprUDlyWm94bmEzb2k3REpkYzJPME1IaTB3M0xVclVQNW80NDQ1?= =?utf-8?B?Vm9JenI4QjBubmtyZkFTNzhydmVEWmMxUmRYV2lGRlRobXBRdUtkSVR0Vk9i?= =?utf-8?B?ZmhCTE4vUlU3dzBaZ0RIeW9HRk1TMEpJRzE2eFFjS0pCMmFDaFFNVnhNQUN5?= =?utf-8?B?YU9LMDNUdFdIUk40c3k2ZzIwZHc1RURYWG4rcGl4c1grN2NRRGRFRFlUV0lr?= =?utf-8?B?MG1Sclg1aU5EU0lvL2c3T0FNVy9NVXVvTmNvVUNwc0J4UDZTL0FMKzB2a3B2?= =?utf-8?B?UktIeGZsZTd3ZTNkY3hDVDc5anI5ZkZZL0pmRTlUajU3TUlISFVOTjBwTmtv?= =?utf-8?B?OU5ScEErdTZtZUNCY2haSzViVGNudVA3bFU4Rnp6ZHA1RWRCKzRWUFBSNzk0?= =?utf-8?B?THpmVWlNaFY2ZHgxN3lreGdCd2xqSXZXTytwNWorVGlvRjJ3ZnJrbUpxMUdR?= =?utf-8?B?MVQxS1NLTlROQldqWC81cUhYaTNCVGxTUzVMUE5lbE1XY2JIempITHRTUk80?= =?utf-8?B?NjFwVFlQeXBOMGdoTEV4MzNRQ3lHZWNLTXg1VHllbU9TVVc2a29OVkhzUkRH?= =?utf-8?B?bi84WFpQdFBaYXNSeTIrYjJzSTFXZndadnpNL255UzFVR1lLRENNbkdsSjZx?= =?utf-8?B?V2NhcEs3NU1zSWRIMlpZeGRzOUFSTzhZRngyL0hRSW5EZGV2VGcxMkh6SzYx?= =?utf-8?B?aDJoV3VUYVRCZHhlYU1aR25NNU9hUndtNCt3UXdSSzdWaWtVKzdENWZ2bllt?= =?utf-8?B?OU92TzdZUTliZ3Nia2hvcTVPTXVKU3A2K2F6cFlRUXAxcmtIK0JpMTdtY0Ja?= =?utf-8?B?NVhiUU9lcXVLVFdYUTV1LzhoUDRKWVRZVnZKNlhnOFBLT3o4MGlmS05NS0Fu?= =?utf-8?B?ZlA2amVDajV0eUxvajBNWmkrbTczZlpDS3FxMzArQUpBdGhkRWpZQkZza1NC?= =?utf-8?B?cm0wQnQ0WDYxNFBMYVA1MGlRN0svQzM1a2xoNFBONFhoTkViS0tzclJOS2Nu?= =?utf-8?B?bFZSVmwxWjRSNThQRHg1Z243NWdxQm1IeVh4dnU2aE5IOUtjQzdoU1VzRWcz?= =?utf-8?B?Vnk3UzVKMlNGd3I0K251V3NML25UeWdYYnBtWFVJL2h3b0dZZWovZ1pXaGo4?= =?utf-8?B?M3R1Zm8zcVA2ajBpKy9JK21yd3FvNmtXZnNXNlNCTlY2SUI2dDZnRmJ5anhG?= =?utf-8?B?TlI3L2g3d0gxa2kvVHBIaHhLRTZFalBDbmduMGJ4S01XN1VwSnUrbnFoU0Vr?= =?utf-8?B?QkdGd2tIL1BBVWxQSmFtandabU10WG5DVWRPUWlMMG1CVDFVNkYxZDJWbFpO?= =?utf-8?B?K21GNEozWlRZLy9XMi9Wb0h3eWlWU3o1djg2MkRmZ2dkTVNuSDd0TXJtSnFv?= =?utf-8?B?SzZ4RjhUK3lQSi9XTXB3RVIwMmZGSFNndDNoekRsS1piY1JjdHl3OFlxNFlZ?= =?utf-8?B?L1NZSDI4Qi9kQlV4WFRpYXI3QmRmK2tuOW9ZRVhudy9zY21uejI3eVJZNW9N?= =?utf-8?B?TTNqaGszTmpTdnRzbUs5Vk95czhMSHp5bWtJWVFXZjZNam9CY0pORXFEQll5?= =?utf-8?B?T0Y4VDU1cWp4aFJnV3VjU3M4OExiTGdHS2VKZkdOUHArUnRUdlFwODNRNlJ0?= =?utf-8?Q?JyA8=3D?= X-Microsoft-Antispam-Message-Info: CmP7jsOK7Fxz4nJhd7qAk0s5aYj897t2abg3B3soyw95RHHC680l4nUbEyAkH2gAefjNRUYu0P3/2XqNbwXSndHZIofZnp3nU7ZryfRJ7wMuowEEhT+BHRrBk8fjzD1nQ6l0D1MyXymeq3lp8BDi2e3rNeJiUWtLv3SgOl+8xRR5B0kHTsUUdS12/uF43qra X-Microsoft-Exchange-Diagnostics: 1;AM3PR08MB0215;6:LLA/RfxOMX9aCntbsLDFhNBi3hK25Eb3HPSRLO0J0hm5sfacxbJaI15+v/WuSpcwdEclnImPoxCjjDfsBcwG6kLJbShQpxdPt3OzLQ4++Rm2MlfVxqR/AxN0VIKWHuzIRXghtLST6Dt2KlQKzjDHdAcjWFmJ/3Uy1b2AkQ6wLWJUEOnyByS9+BbslbyTdlkPq2F2eiQz5LAK8l6p5kjN2rCx4Vovh7Uyz2z+CwzPZJ5/0WdviRNAD2IlFAylizzkvT4M0NnvTizEMFwLBTiZ0vllnrKfoUrBhuHyHBo/DQSzv7S2PB7mz3YTqKjNc9B9gF3Qxg9/N5/7/Ecy+fFOL4k8A4ZtcdAyx7MHpAjuNnk=;5:/vVT0KrJ7LfuuD1icayg/zBUfXuAPpiAqHp1yyNVTbc90NQYoyX3Ym9m5q2n3FKG+2zaYax3MAMDd73oOhINwgCw5Nt0Da/UfwT5bc30JRR+eHvMlbsy1W0Qz9N1dkESgk32m30PMBgGXT17UmnM0ycqHI1PO/bPb4CzClG/y38=;24:FXsc205VGw4081LoRaAPanwyh2n4Y1jKdgr7U7wnlhyEpgDTJ7VbXfmNz4m48dmmTflpeQmGF5pJSjKC3iqKMPrFGlNp6+TAzrBqZpEogDk=;7:lES8WwaYbHZjkj07yEGsAPjHemdzs+E+Qyfq5pwN6jAGlTFluYQxWis2gaqYXDQCLTJ9jtRO+HrY0BuSW+bdEv4RNfs3LcAPCXzGcEeTapIRPKXEE9JDNZP0so2emyI6pl31HjJAa0Wp/ypRBFzt6+nr2xHhRkqyA5TueV4YKAHnlXvBBeWaUWHSeIK7iGql/YB97jAhQOHG8WCQaMBdJf27Ai+3pLwc81QUh89q+ZJPUrU+yACurIGLwb01OI8o SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Mar 2018 13:36:23.4874 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: a042282a-a556-4077-968d-08d58ff9e79b X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR08MB0215 X-IsSubscribed: yes X-SW-Source: 2018-03/txt/msg01183.txt.bz2 Hi Sameera On 22/03/18 02:07, Sameera Deshpande wrote: > Hi Sudakshina, > > As per the ARMv8 ARM, for the offset range (-1048576 ,1048572), the > far branch instruction offset is inclusive of both the offsets. Hence, > I am using <=||=> and not <||>= as it was in previous implementation. I have to admit earlier I was only looking at the patch mechanically and found a difference with the previous implementation in offset comparison. After you pointed out, I looked up the ARMv8 ARM and I have a couple of doubts: 1. My understanding is that any offset in [-1048576 ,1048572] both inclusive qualifies as an 'in range' offset. However, the code for both attribute length and far_branch has been using [-1048576 ,1048572), that is, ( >= && < ). If the far_branch was incorrectly calculated, then maybe the length calculations with similar magic numbers should also be corrected? Of course, I am not an expert in this and maybe this was a conscience decision so I would ask Ramana to maybe clarify if he remembers. 2. Now to come back to your patch, if my understanding is correct, I think a far_branch would be anything outside of this range, that is, (offset < -1048576 || offset > 1048572), anything that can not be represented in the 21-bit range. Thanks Sudi > > On 16 March 2018 at 00:51, Sudakshina Das wrote: >> On 15/03/18 15:27, Sameera Deshpande wrote: >>> >>> Ping! >>> >>> On 28 February 2018 at 16:18, Sameera Deshpande >>> wrote: >>>> >>>> On 27 February 2018 at 18:25, Ramana Radhakrishnan >>>> wrote: >>>>> >>>>> On Wed, Feb 14, 2018 at 8:30 AM, Sameera Deshpande >>>>> wrote: >>>>>> >>>>>> Hi! >>>>>> >>>>>> Please find attached the patch to fix bug in branches with offsets over >>>>>> 1MiB. >>>>>> There has been an attempt to fix this issue in commit >>>>>> 050af05b9761f1979f11c151519e7244d5becd7c >>>>>> >>>>>> However, the far_branch attribute defined in above patch used >>>>>> insn_length - which computes incorrect offset. Hence, eliminated the >>>>>> attribute completely, and computed the offset from insn_addresses >>>>>> instead. >>>>>> >>>>>> Ok for trunk? >>>>>> >>>>>> gcc/Changelog >>>>>> >>>>>> 2018-02-13 Sameera Deshpande >>>>>> * config/aarch64/aarch64.md (far_branch): Remove attribute. >>>>>> Eliminate >>>>>> all the dependencies on the attribute from RTL patterns. >>>>>> >>>>> >>>>> I'm not a maintainer but this looks good to me modulo notes about how >>>>> this was tested. What would be nice is a testcase for the testsuite as >>>>> well as ensuring that the patch has been bootstrapped and regression >>>>> tested. AFAIR, the original patch was put in because match.pd failed >>>>> when bootstrap in another context. >>>>> >>>>> >>>>> regards >>>>> Ramana >>>>> >>>>>> -- >>>>>> - Thanks and regards, >>>>>> Sameera D. >>>> >>>> >>>> The patch is tested with GCC testsuite and bootstrapping successfully. >>>> Also tested for spec benchmark. >>>> >> >> I am not a maintainer either. I noticed that the range check you do for >> the offset has a (<= || >=). The "far_branch" however did (< || >=) for a >> positive value. Was that also part of the incorrect offset calculation? >> >> @@ -692,7 +675,11 @@ >> { >> if (get_attr_length (insn) =3D=3D 8) >> { >> - if (get_attr_far_branch (insn) =3D=3D 1) >> + long long int offset; >> + offset =3D INSN_ADDRESSES (INSN_UID (XEXP (operands[2], 0))) >> + - INSN_ADDRESSES (INSN_UID (insn)); >> + >> + if (offset <=3D -1048576 || offset >=3D 1048572) >> return aarch64_gen_far_branch (operands, 2, "Ltb", >> "\\t%0, %1, "); >> else >> @@ -709,12 +696,7 @@ >> (if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int >> -32768)) >> (lt (minus (match_dup 2) (pc)) (const_int >> 32764))) >> (const_int 4) >> - (const_int 8))) >> - (set (attr "far_branch") >> - (if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int >> -1048576)) >> - (lt (minus (match_dup 2) (pc)) (const_int >> 1048572))) >> - (const_int 0) >> - (const_int 1)))] >> + (const_int 8)))] >> >> ) >> >> Thanks >> Sudi >> >>>> -- >>>> - Thanks and regards, >>>> Sameera D. >>> >>> >>> >>> >> > > >