From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51802 invoked by alias); 29 Mar 2018 15:31:22 -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 50763 invoked by uid 89); 29 Mar 2018 15:31:21 -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=H*M:f982 X-HELO: EUR02-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr20086.outbound.protection.outlook.com (HELO EUR02-VE1-obe.outbound.protection.outlook.com) (40.107.2.86) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 29 Mar 2018 15:31:19 +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 DB3PR08MB0217.eurprd08.prod.outlook.com (2a01:111:e400:504d::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.609.10; Thu, 29 Mar 2018 15:31:15 +0000 Subject: Re: [Aarch64] Fix conditional branches with target far away. To: Sameera Deshpande Cc: Ramana Radhakrishnan , 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, 29 Mar 2018 15:33: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: AM4P190CA0015.EURP190.PROD.OUTLOOK.COM (2603:10a6:200:56::25) To DB3PR08MB0217.eurprd08.prod.outlook.com (2a01:111:e400:504d::27) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: bad30545-23c6-4bfd-b7be-08d5958a1c71 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(2017052603328)(7153060)(7193020);SRVR:DB3PR08MB0217; X-Microsoft-Exchange-Diagnostics: 1;DB3PR08MB0217;3:QZ9QWr2N+0Lt9qsnafl+tHXaKFQAOSgkVPbxulrovxG4+UKvNwQ4PSuaydVwjpRmCOzv+Ki7OEonmDSFnpd4qs457q1iGiOHZgJiYGQVxLBG5xrtfAurWge/zIgOGhxZS0ELDuyjwQotyJfxlh6A62gCqF3cuXj2FGVTFM7lQqqcqbobsDoOsMS3aGWCKIKiRzWsMSsq6tDBvdrr69lMXqY3nmb1WprNmb14QgYRKNlpEXpmT1cD2/sidTPVNtJ9;25:LquwzV836FUGpdguGf2NwIPcfmAHugunSynl3kXjAJ1HhVo9+qa2EX+2WIXXAnsTqrU07lEmqCMAPLd7uS9zafjvXOwzHcc9H4E1RPwCewAdx/dcq+O3faKbSDQVBq3W+jrO6ul1wlCpnM5GXj4cLVIfPQQNOCygSFyvak2KpWsyNbSHOIJa7dxQB5ROWrZzFWJnTaIMEHHr7ycEz+bup7HD8CntyuGPQ+zfkPgwLotJuPh6b6Kw7YnhN5T2uaHI/2fhxlxhIjDbl0aR18yLyBwpwXFYZZosab9VyrcPsuJm0114e7A5BPS/RpGpAkdbF+OPa+MJhPo4XKXTDZwY+Q==;31:OhOyJ0EG95ek7Do+lvqMcWhw0vujfZLr/l905t7JfWxdwMfk4rcTQvwATai+fxMK7Hsc7eIboFZEQmEQ4WL9Cc2rJ5IFw535/4oVv0t0DQgGYh7DoY+MMDb6B9hedwThYcmvPrUgRfAM9jn1PlD6MLJ08JU5+ztjt2k5eFPCmNhgihdprMc5QJtoqm9pGj9095uYskvU4QiTBT/8Mr317jODPubpwSgSZ5diHANFqG8= X-MS-TrafficTypeDiagnostic: DB3PR08MB0217: NoDisclaimer: True X-Microsoft-Exchange-Diagnostics: 1;DB3PR08MB0217;20:A4Du+DbPkyz0BcUxneNmo8+GO7rmBxT88OEpxGCDG4BtP842WtFSX8qFE+BDCZMuC52iiEc2vlrG/dB+uro5BS7iNBdG4iWyW1dUBzACK722xKrGGoiixLrChH14MGCSdeEMpYtZusmPwJm4cf7ZexKCYI5A5ioUEf6e6OWrJFA=;4:YJmuAW36TzZHw9BPyTuzFmBieXx64rJJGRXANFiCLMLU3yShxGs+ZRTb0pyQJ6kND/VMop9GWoO3+HGhwdqB0useg6BtpzXkhvkOjKWGEVM/Q/4Yzo6QQGliflVwg8P9MFc0XOb90DG74TjjkDsWVOwVySH9ZSuEc7DqNd/VNK1nPB6Obqm9ujuv+4dgC0CN0XnZIDmw9HNpuiXx2wj10tvpu2DXz1OBRevnkmWQq3iqShGfqQpsqKIF3OCLg2lwq4Ej3dFMLSbnD7b4lXoPsXL8nA46gYE48wN6ZczB31MYZzigH2G8YhnIZj/9hKCw7buGBiaDM0zKVn/ZjJ+sMSftE6taQ8QeLG7RHgJBMDk= 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)(5005006)(8121501046)(93006095)(93001095)(3002001)(10201501046)(3231221)(944501327)(52105095)(6055026)(6041310)(20161123560045)(20161123558120)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(6072148)(201708071742011);SRVR:DB3PR08MB0217;BCL:0;PCL:0;RULEID:;SRVR:DB3PR08MB0217; X-Forefront-PRVS: 0626C21B10 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6049001)(376002)(346002)(39860400002)(39380400002)(396003)(366004)(377424004)(43544003)(199004)(189003)(54534003)(86362001)(7736002)(23676004)(26005)(386003)(2486003)(76176011)(6116002)(16526019)(53546011)(230700001)(11346002)(81156014)(77096007)(81166006)(8676002)(2906002)(478600001)(31686004)(72206003)(106356001)(446003)(67846002)(105586002)(956004)(52116002)(2616005)(3846002)(36756003)(486005)(476003)(52146003)(486005)(8936002)(65956001)(65806001)(6246003)(66066001)(53936002)(93886005)(5890100001)(54906003)(31696002)(58126008)(4326008)(68736007)(50466002)(47776003)(65826007)(64126003)(6916009)(25786009)(229853002)(16576012)(5660300001)(316002)(6486002)(305945005)(97736004)(299355004);DIR:OUT;SFP:1101;SCL:1;SRVR:DB3PR08MB0217;H:[10.2.206.246];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQjNQUjA4TUIwMjE3OzIzOmZiL3I4S0dNdHJDU2RBdmUzOVk4WWl1RGxa?= =?utf-8?B?bGJzeE5qM3hDb1Y1bkdDelBkYXcvNE01U1FQZTVRdUlDZEUxL1c2emZiTFNU?= =?utf-8?B?RFNWVDhRQUxDL0VWL3FlNUk1N2pQK0VqRWY2VUpMekI5d0tNM3FQUTY3QWtP?= =?utf-8?B?MmZZTnkvenloSUd0dEFpSlUxZHFicTRPalZMUnFQbXJmYUFxMzFSWUFMeE05?= =?utf-8?B?ZmFkTUxxb1NCTkZFbTluVGg1Y3NIbEZjbHJzUDlKZDZrYjFVMjlHR1hhbGFl?= =?utf-8?B?OUM0TjY5WE9SNklYRTNrN04xQ0kwQXR3S1kzSkl5OGE1UVVOWTR5dytrbVZi?= =?utf-8?B?bEpBRFZQVlVBRmx6Y0c3QWQwQ1RFQnVsWTVRR1dqbFYvVUFOaUxnN1hXb2ZE?= =?utf-8?B?WEhNZFdmV3llWVQxTHQ5S2I0OHJ1eDUrM1pBdmNWdEF4RVIyaUxBdWtyQitX?= =?utf-8?B?UWJ4STN3dFlMUklKYWR5SXlhdCtCZWtBdHJDSGJYSVFMSytnM2d3YzI4S1FS?= =?utf-8?B?WUdWUExsL2FoYjVlb0VMQ2lrdGx3M3NzampkS2tLLzBWMVppTjF4TmJvUU1E?= =?utf-8?B?Zk1JRDYrMFMzRUJtWThERXhzVTFtQXp6b015WkdSa2xYb040aG10bU5ORm9u?= =?utf-8?B?SUdFOHlIWWh1L0ZDalhQQWcwd0o2aUZhZUF1b0w2c2E0WHo4VnFGY2ZsekRW?= =?utf-8?B?Ui9uN1JuZDh2UUp2aWEyNm9UWkVtRWZvOE9NSGZLaURSdHdwODNPUHduUmJU?= =?utf-8?B?NUYvT0Vzc2JGTGtQcGE3WHdTZ2lTWHJYT3R1c2xlbEJ4RDI4ZzZ5NzBTSjZV?= =?utf-8?B?U0cyRFBtQmw5VW5FODRnUktnNHFTNGtxamVsQ2M4NjRML29GRkQ4eDN1eHFS?= =?utf-8?B?Z0xmb3pRWTdUNUdWclNZOXJlcmVxcFJqQVBYdk44R3hIUmt0Y0JWVTNiWm1q?= =?utf-8?B?eGVVUG1LdTJtM0d2ZTlTdDA0OXpXWnR6SjJRZ3lpWW42VERVMDhRanJwQ0Vo?= =?utf-8?B?SHd4dVVVcHBBNWhFd2VoQnY2KzhGK3R4YnVWVnBoUVJSMCtheGh5OUQ1UWc0?= =?utf-8?B?Q0x5ODF0a1JHQlY1YUIvTmFwSDhzNmlxTXNkelZuT3VUajN3WDBxbk1qaGdt?= =?utf-8?B?ZVhZa1B1eDZ5QmNFTjUzNVhhWEU2elkyVnVSenZTenYvb2pqZzdpQ0dXd2dT?= =?utf-8?B?c2tybndBK1Q5MjQzSlFxeWFBUW9DQWNHTnR1am1FOUxuQUcwaG13Z0xXSHp2?= =?utf-8?B?TWNqQlcyNEl0KzYzY0wvdEJVR1RsUGxDWS81VkQ5UDg4MG9PZmloMTgvMDVw?= =?utf-8?B?SWErNXNyUnpHZGFzRmtsU3Babk1HaFBWKzBkdHV1SzYyQVp4NGxhRlIwbEVk?= =?utf-8?B?RFNLd2oyTzJEdm9vSGpmdWo1ZXJiRlp2ZUhBNDFvbng3dUZqZFhuczg2Y0hH?= =?utf-8?B?d2RpaFZjb2xHOXJPWTZibkRXeS9SWlhXOWNISjFSTGxyTHFtWUpmanFlYUhw?= =?utf-8?B?UnFKcjJ0VzZaU1JUMTFVd1B2TFcyYksrMXhSVDBsbERjZmMwZVliZVZCN0Jw?= =?utf-8?B?QVZhNnhweVBpMmYzcTlqSDk2Y0FIT0lKdW1laE1HM0dvN3lpb0YzNm5pMDFU?= =?utf-8?B?RnNPVU5MMVBpdnJJdm1BdmsyNTRxVzBnRHk4UHdQd1ZHYzlvQjlZcW5objBh?= =?utf-8?B?OS9pc2ZwVTFQQzZSUWZYcGFtUEhCVEFUVTFEUy9NelBaUGJ5Y0pVeDdiWFcz?= =?utf-8?B?bkFlM0lBUXhURjRuMExVeDcrTzc3MHBkcm1kazVpclhBTElCbTd2bjBSY1Ux?= =?utf-8?B?bWFnQmY1U0VHUWdzRTdvUkZLSzlLV24yOGdKbzZPMHVRNUdMTzB5VDNUMGtw?= =?utf-8?B?QUJEem1XMXlnSS9EQkhOZ2VvSjFPb0FEZFV4bno3d2N0TzVJTGtBS3FWUDFQ?= =?utf-8?B?M3llbEkvaDFyRHF0UVVXSXFFOXRRMWErYlBGWFBVZzFoM3RMTlprZTJHQ0JG?= =?utf-8?B?dFo1VU8wWmlTa2R6a1M4UlBaRTFKNWtYZU80bCsvM2NpZm01c3NQUS85Zm1z?= =?utf-8?B?OEg1MTFCWFR5RE5jVkJuMWNqL1NMNkJxQi9TV2lUQktGMHdoMGRac0JZR1kx?= =?utf-8?Q?Fh8Sl0dK6w+gx499w8eDW1GXdPl9zudkgSaKVYsugOAX?= X-Microsoft-Antispam-Message-Info: wzBiaWFffi90zvGezQU8Fpn82M6r4GjOXq6hNTfsVPdAUKqiQZhmppmhNO53JcxBqiyR9ATeePdrzzOX6S/fw7QLNjZygzTek2sL5r6/IshpZf+4SDYrOxxzaIO2XgPTbLNVPJCeC8NEyU1qVG/9n6B2kbk5VlRVNg5MbAvQEXEp5k/B5muElIWBpIFzGDON X-Microsoft-Exchange-Diagnostics: 1;DB3PR08MB0217;6:2UJjG1fe/6qKj4oxHcUQGtphCprOB/FejEeQanOD+hor00NR3SCGrfGcWDngsfabxiYDOPHK6nSlJGYSPkIG7WBu6lBkESz6rUdW1MfUuxXD7PryyUyay+Lv4bfAqCnHeaJ9qGyBfddl01+k6xjOQh54x0oXstf7hCT6X4r2IYpXOrPPiryZrz3cSLTRRFKZ473Brwl4the61anA5/b+ESAZpxSteyrpx0jkA7nMpL2wuVAaAnVcqdxfyoowkpdApli61FV4ajwE/gKGrUi2xE3ZdRJs9XLl0dJM9Y5qaWHlGs6K8ZksVfdWltVaVG12MByTQSJBfBNBAJaHQZIgS8QCQoj0WC/HgZ8mPdOfFGCnNe7zw5Dl3qq9cSVaIYWo8LhG/rWMelBpyO/5Q5BvEaxutZ55w7+q9T0kjRl629f3mNwS+mEd1oWYVZdfigJy+eXTz4pxRsPc0mszoB0+JA==;5:zi7OXnDFCpnkLchbc9XQeyW1MpEd4mh8IHEtzhpuQiaCU2mxgw4zzpBZAGCoBkRi+yr9eItlCKjxmW6TbBQVQ5KnPirqpqdYAlIMZu1UAg9H9TiEOdHExPwymSBd1UMmnbpoFtcm6p4inQZO4RE1MZaLvj00lSvU/1nRxBpfY/E=;24:11vWqmInWlbM0Htwx9JhgYm9JZPH4XwHO5FlbyVFcQmqUKJ+glyDk2Et+Q9GKYGJk251gAfQjcUXQ4ByuBFcfR4iCRL1PVxRGiu6HIYyVGg= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DB3PR08MB0217;7:C39qpyx2CXLVGQhySS8YcFpDNMIWq0R5Vf4QER5Umf+0xlevvbHT+4qxCKj0pZfzSylRHqRfIBtALUVmZvWUHvnCx750S90ss9IlFYCP2O/UF71IrrfXdvje45GcJDeu1VhIDRp4GJ8bp08opsBOexwLMPqHEIoFuhFf178L7P1P6qQmEWWDlE+JUbJphvwaWLls1PVY5tumvLSa73CrFnSfFBM9/MURuBkwxWywdGzTST144irRToe8lwyqLZwZ X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Mar 2018 15:31:15.3814 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: bad30545-23c6-4bfd-b7be-08d5958a1c71 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR08MB0217 X-IsSubscribed: yes X-SW-Source: 2018-03/txt/msg01551.txt.bz2 Hi Sameera On 29/03/18 11:44, Sameera Deshpande wrote: > Hi Sudakshina, > > Thanks for pointing that out. Updated the conditions for attribute > length to take care of boundary conditions for offset range. > > Please find attached the updated patch. > > I have tested it for gcc testsuite and the failing testcase. Ok for trunk? Thank you so much for fixing the length as well along with you patch. You mention a failing testcase? Maybe it would be helpful to add that to the patch for the gcc testsuite. Sudi > > On 22 March 2018 at 19:06, Sudakshina Das wrote: >> 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. >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>> >>> >>> >> > > >