From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 77166 invoked by alias); 8 Jun 2018 00:19:17 -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 77155 invoked by uid 89); 8 Jun 2018 00:19:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL,BAYES_50,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=separates, UD:aarch64.md, aarch64.md, aarch64md X-HELO: EUR01-DB5-obe.outbound.protection.outlook.com Received: from mail-db5eur01on0047.outbound.protection.outlook.com (HELO EUR01-DB5-obe.outbound.protection.outlook.com) (104.47.2.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Jun 2018 00:19:13 +0000 Received: from DB5PR08CA0024.eurprd08.prod.outlook.com (2a01:111:e400:52c3::34) by DB5PR0801MB1589.eurprd08.prod.outlook.com (2603:10a6:0:3b::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.841.14; Fri, 8 Jun 2018 00:19:09 +0000 Received: from VE1EUR03FT035.eop-EUR03.prod.protection.outlook.com (2a01:111:f400:7e09::205) by DB5PR08CA0024.outlook.office365.com (2a01:111:e400:52c3::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.841.14 via Frontend Transport; Fri, 8 Jun 2018 00:19:09 +0000 Authentication-Results: spf=pass (sender IP is 217.140.96.140) smtp.mailfrom=arm.com; gcc.gnu.org; dkim=none (message not signed) header.d=none;gcc.gnu.org; dmarc=bestguesspass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 217.140.96.140 as permitted sender) receiver=protection.outlook.com; client-ip=217.140.96.140; helo=nebula.arm.com; Received: from nebula.arm.com (217.140.96.140) by VE1EUR03FT035.mail.protection.outlook.com (10.152.18.110) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA_P384) id 15.20.841.10 via Frontend Transport; Fri, 8 Jun 2018 00:19:09 +0000 Received: from arm.com (10.1.2.79) by mail.arm.com (10.1.105.66) with Microsoft SMTP Server id 14.3.294.0; Fri, 8 Jun 2018 01:19:04 +0100 Date: Fri, 08 Jun 2018 00:19:00 -0000 From: James Greenhalgh To: Michael Collison CC: GCC Patches , nd Subject: Re: [PATCH][Aarch64] v2: Arithmetic overflow common functions [Patch 1/4] Message-ID: <20180608001902.GA24135@arm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DB5PR0801MB1589: NoDisclaimer: True X-Exchange-Antispam-Report-Test: UriScan:(180628864354917)(22074186197030)(183786458502308); X-MS-Exchange-SenderADCheck: 1 X-Forefront-PRVS: 06973FFAD3 SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Office365-Filtering-Correlation-Id: 42144d78-3877-4665-6456-08d5ccd57447 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jun 2018 00:19:09.1073 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 42144d78-3877-4665-6456-08d5ccd57447 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[217.140.96.140];Helo=[nebula.arm.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB5PR0801MB1589 X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00427.txt.bz2 On Wed, Jun 06, 2018 at 12:14:03PM -0500, Michael Collison wrote: > This is a respin of a AArch64 patch that adds support for builtin arithmetic overflow operations. This update separates the patch into multiple pieces and addresses comments made by Richard Earnshaw here: > > https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00249.html > > Original patch and motivation for patch here: > > https://gcc.gnu.org/ml/gcc-patches/2017-05/msg01512.html > > This patch primarily contains common functions in aarch64.c for generating TImode scratch registers, > and common rtl functions utilized by the overflow patterns in aarch64.md. In addition a new mode representing overflow CC_Vmode is introduced. > > Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk? Normally it is preferred that each patch in a series stands independent of the others. So if I apply just 1/4 I should get a working toolchain. You have some dependencies here between 1/4 and 3/4. Rather than ask you to rework these patches, I think I'll instead ask you to squash them all to a single commit after we're done with review. That will save you some rebase work and maintain the property that trunk can be built at most revisions. > (aarch64_add_128bit_scratch_regs): Declare > (aarch64_subv_128bit_scratch_regs): Declare. Why use 128bit in the function name rather than call it aarch64_subvti_scratch_regs ? > @@ -16337,6 +16353,131 @@ aarch64_split_dimode_const_store (rtx dst, rtx src) > return true; > } > > +/* Generate RTL for a conditional branch with rtx comparison CODE in > + mode CC_MODE. The destination of the unlikely conditional branch > + is LABEL_REF. */ > + > +void > +aarch64_gen_unlikely_cbranch (enum rtx_code code, machine_mode cc_mode, > + rtx label_ref) > +{ > + rtx x; > + x = gen_rtx_fmt_ee (code, VOIDmode, > + gen_rtx_REG (cc_mode, CC_REGNUM), > + const0_rtx); > + > + x = gen_rtx_IF_THEN_ELSE (VOIDmode, x, > + gen_rtx_LABEL_REF (VOIDmode, label_ref), > + pc_rtx); > + aarch64_emit_unlikely_jump (gen_rtx_SET (pc_rtx, x)); > +} > + I'm a bit surprised this is AArh64 specific and there are no helper functions to get you here. Not that it should block the patch;l but if we can reuse something I'd prefer we did. > +void > +aarch64_expand_subvti (rtx op0, rtx low_dest, rtx low_in1, > + rtx low_in2, rtx high_dest, rtx high_in1, > + rtx high_in2) > +{ > + if (low_in2 == const0_rtx) > + { > + low_dest = low_in1; > + emit_insn (gen_subdi3_compare1 (high_dest, high_in1, > + force_reg (DImode, high_in2))); > + } > + else > + { > + if (CONST_INT_P (low_in2)) > + { > + low_in2 = force_reg (DImode, GEN_INT (-UINTVAL (low_in2))); > + high_in2 = force_reg (DImode, high_in2); > + emit_insn (gen_adddi3_compareC (low_dest, low_in1, low_in2)); > + } > + else > + emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2)); > + emit_insn (gen_subdi3_carryinCV (high_dest, > + force_reg (DImode, high_in1), > + high_in2)); This is where we'd break the build. gen_subdi3_carryinCV isn't defined until 3/4. The above points are minor. This patch is OK with them cleaned up, once I've reviewed the other 3 parts to this series. James > > 2018-05-31 Michael Collison > Richard Henderson > > * config/aarch64/aarch64-modes.def (CC_V): New. > * config/aarch64/aarch64-protos.h > (aarch64_add_128bit_scratch_regs): Declare > (aarch64_subv_128bit_scratch_regs): Declare. > (aarch64_expand_subvti): Declare. > (aarch64_gen_unlikely_cbranch): Declare > * config/aarch64/aarch64.c (aarch64_select_cc_mode): Test > for signed overflow using CC_Vmode. > (aarch64_get_condition_code_1): Handle CC_Vmode. > (aarch64_gen_unlikely_cbranch): New function. > (aarch64_add_128bit_scratch_regs): New function. > (aarch64_subv_128bit_scratch_regs): New function. > (aarch64_expand_subvti): New function.