On Sat, Jan 4, 2020 at 12:39 PM Jakub Jelinek <ja...@redhat.com> wrote: > > On Sat, Jan 04, 2020 at 12:13:50PM +0100, Uros Bizjak wrote: > > LGTM, but I wonder if *addcarry<mode>_1 gets overmacroized, the insn > > condition is really hard to comprehend. Perhaps it should be written > > as a separate pattern for SImode and DImode instead? > > > > +(define_insn "*addcarry<mode>_1" > > > + [(set (reg:CCC FLAGS_REG) > > > + (compare:CCC > > > + (zero_extend:<DWI> > > > + (plus:SWI48 > > > + (plus:SWI48 > > > + (match_operator:SWI48 5 "ix86_carry_flag_operator" > > > + [(match_operand 3 "flags_reg_operand") (const_int 0)]) > > > + (match_operand:SWI48 1 "nonimmediate_operand" "%0")) > > > + (match_operand:SWI48 2 "x86_64_immediate_operand" "e"))) > > > + (plus:<DWI> > > > + (match_operand:<DWI> 6 "const_scalar_int_operand" "") > > > + (match_operator:<DWI> 4 "ix86_carry_flag_operator" > > > + [(match_dup 3) (const_int 0)])))) > > > + (set (match_operand:SWI48 0 "register_operand" "=r") > > > + (plus:SWI48 (plus:SWI48 (match_op_dup 5 > > > + [(match_dup 3) (const_int 0)]) > > > + (match_dup 1)) > > > + (match_dup 2)))] > > > + "ix86_binary_operator_ok (PLUS, <MODE>mode, operands) > > > + && CONST_INT_P (operands[2]) > > > + /* Check that operands[6] is operands[2] zero extended from > > > + <MODE>mode to <DWI>mode. */ > > > + && ((<MODE>mode == SImode || INTVAL (operands[2]) >= 0) > > > + ? (CONST_INT_P (operands[6]) > > > + && UINTVAL (operands[6]) == (UINTVAL (operands[2]) > > > + & GET_MODE_MASK (<MODE>mode))) > > > + : (CONST_WIDE_INT_P (operands[6]) > > > + && CONST_WIDE_INT_NUNITS (operands[6]) == 2 > > > + && ((unsigned HOST_WIDE_INT) CONST_WIDE_INT_ELT (operands[6], 0) > > > + == UINTVAL (operands[2])) > > > + && CONST_WIDE_INT_ELT (operands[6], 1) == 0))" > > I'm afraid it wouldn't help. For SImode it would remove the : part, sure, > but any optimizing compiler will optimize that away too, for DImode we need > both branches - if INTVAL isn't negative, its zero extension will still be > a CONST_INT with the same value, if it is negative, zero extension will be a > CONST_WIDE_INT. It would be shorter to write > rtx_equal_p (operands[6], > simplify_unary_operation (ZERO_EXTEND, <DWI>mode, operands[2], > <MODE>mode)) > but I wanted to avoid that because it is quite costly and will create a new > rtx each time the condition is checked. > Anyway, attached is in one file the *addcarry<mode>_1 from the patch and > in another one the demacroized variant, the macroized one is 41 lines > long, the demacroized one is 75 lines long, so not fully 2 times, but > almost. Yes, in the SImode case we can just use const_int_operand and don't > need to test it in the condition.
Thanks for the examples! Yes, I agree that the expanded version is not much better than macroized (it is even worse...). So, OK. Thanks, Uros.
