All requested changes made: - label_ref added as operand 3 - more meaningful names given to variables
Okay for trunk? -----Original Message----- From: James Greenhalgh <james.greenha...@arm.com> Sent: Thursday, June 7, 2018 5:29 PM To: Michael Collison <michael.colli...@arm.com> Cc: GCC Patches <gcc-patches@gcc.gnu.org>; nd <n...@arm.com> Subject: Re: [PATCH][Aarch64] v2: Arithmetic overflow addv patterns [Patch 2/4] On Wed, Jun 06, 2018 at 12:16:22PM -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 contains new patterns for addv<mode> overflow patterns. > > Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk? > +(define_expand "addv<mode>4" > + [(match_operand:GPI 0 "register_operand") > + (match_operand:GPI 1 "register_operand") > + (match_operand:GPI 2 "register_operand") > + (match_operand 3 "")] > + "" It won't be validated; but I'd prefer us to add the constraint on the label so this code is self-documenting. It would have saved me a trip to the manual to understand operand 3. > (define_expand "addti3" > [(set (match_operand:TI 0 "register_operand" "") > (plus:TI (match_operand:TI 1 "register_operand" "") > - (match_operand:TI 2 "register_operand" "")))] > + (match_operand:TI 2 "aarch64_reg_or_imm" "")))] > "" > { > - rtx low = gen_reg_rtx (DImode); > - emit_insn (gen_adddi3_compareC (low, gen_lowpart (DImode, operands[1]), > - gen_lowpart (DImode, operands[2]))); > + rtx l0,l1,l2,h0,h1,h2; Let's give these slightly meaningful names please. dest_high, dest_low, op1_high, etc. Other than these two comments, I think this is OK. There are some subtleties in here though that I've probably missed, so I wouldn't say no to a second pair of eyes. Thanks, James > > > 2018-05-31 Michael Collison <michael.colli...@arm.com> > Richard Henderson <r...@redhat.com> > > * config/aarch64/aarch64.md: (addv<GPI>4, uaddv<GPI>4): New. > (addti3): Create simpler code if low part is already known to be 0. > (addvti4, uaddvti4): New. > (*add<GPI>3_compareC_cconly_imm): New. > (*add<GPI>3_compareC_cconly): New. > (*add<GPI>3_compareC_imm): New. > (*add<GPI>3_compareC): Rename from add<GPI>3_compare1; do not > handle constants within this pattern.. > (*add<GPI>3_compareV_cconly_imm): New. > (*add<GPI>3_compareV_cconly): New. > (*add<GPI>3_compareV_imm): New. > (add<GPI>3_compareV): New. > (add<GPI>3_carryinC, add<GPI>3_carryinV): New. > (*add<GPI>3_carryinC_zero, *add<GPI>3_carryinV_zero): New. > (*add<GPI>3_carryinC, *add<GPI>3_carryinV): New. > ((*add<GPI>3_compareC_cconly_imm): Replace 'ne' operator > with 'comparison' operator. > (*add<GPI>3_compareV_cconly_imm): Ditto. > (*add<GPI>3_compareV_cconly): Ditto. > (*add<GPI>3_compareV_imm): Ditto. > (add<GPI>3_compareV): Ditto. > (add<mode>3_carryinC): Ditto. > (*add<mode>3_carryinC_zero): Ditto. > (*add<mode>3_carryinC): Ditto. > (add<mode>3_carryinV): Ditto. > (*add<mode>3_carryinV_zero): Ditto. > (*add<mode>3_carryinV): Ditto.
gnutools-6308-pt2.patch
Description: gnutools-6308-pt2.patch
