[PATCH] Improve __builtin_add_overflow on x86 for double-word types (PR target/93141)

Jakub Jelinek jakub@redhat.com
Sat Jan 4 11:39:00 GMT 2020


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.

	Jakub
-------------- next part --------------
(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))"
  "adc{<imodesuffix>}\t{%2, %0|%0, %2}"
  [(set_attr "type" "alu")
   (set_attr "use_carry" "1")
   (set_attr "pent_pair" "pu")
   (set_attr "mode" "<MODE>")
   (set (attr "length_immediate")
     (if_then_else (match_test "IN_RANGE (INTVAL (operands[2]), -128, 127)")
       (const_string "1")
       (const_string "4")))])
-------------- next part --------------
(define_insn "*addcarrydi_1"
  [(set (reg:CCC FLAGS_REG)
	(compare:CCC
	  (zero_extend:TI
	    (plus:DI
	      (plus:DI
		(match_operator:DI 5 "ix86_carry_flag_operator"
		  [(match_operand 3 "flags_reg_operand") (const_int 0)])
		(match_operand:DI 1 "nonimmediate_operand" "%0"))
	      (match_operand:DI 2 "x86_64_immediate_operand" "e")))
	  (plus:TI
	    (match_operand:TI 6 "const_scalar_int_operand" "")
	    (match_operator:TI 4 "ix86_carry_flag_operator"
	      [(match_dup 3) (const_int 0)]))))
   (set (match_operand:DI 0 "register_operand" "=r")
	(plus:DI (plus:DI (match_op_dup 5 [(match_dup 3) (const_int 0)])
			  (match_dup 1))
		 (match_dup 2)))]
  "TARGET_64BIT
   && ix86_binary_operator_ok (PLUS, DImode, operands)
   && CONST_INT_P (operands[2])
   /* Check that operands[6] is operands[2] zero extended from
      DImode to TImode.  */
   && (INTVAL (operands[2]) >= 0)
       ? (CONST_INT_P (operands[6])
	  && UINTVAL (operands[6]) == UINTVAL (operands[2]))
       : (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))"
  "adc{q}\t{%2, %0|%0, %2}"
  [(set_attr "type" "alu")
   (set_attr "use_carry" "1")
   (set_attr "pent_pair" "pu")
   (set_attr "mode" "DI")
   (set (attr "length_immediate")
     (if_then_else (match_test "IN_RANGE (INTVAL (operands[2]), -128, 127)")
       (const_string "1")
       (const_string "4")))])

(define_insn "*addcarrysi_1"
  [(set (reg:CCC FLAGS_REG)
	(compare:CCC
	  (zero_extend:DI
	    (plus:SI
	      (plus:SI
		(match_operator:SI 5 "ix86_carry_flag_operator"
		  [(match_operand 3 "flags_reg_operand") (const_int 0)])
		(match_operand:SI 1 "nonimmediate_operand" "%0"))
	      (match_operand:SI 2 "x86_64_immediate_operand" "e")))
	  (plus:DI
	    (match_operand:DI 6 "const_int_operand" "")
	    (match_operator:DI 4 "ix86_carry_flag_operator"
	      [(match_dup 3) (const_int 0)]))))
   (set (match_operand:SI 0 "register_operand" "=r")
	(plus:SI (plus:SI (match_op_dup 5 [(match_dup 3) (const_int 0)])
			  (match_dup 1))
		 (match_dup 2)))]
  "!TARGET_64BIT
   && ix86_binary_operator_ok (PLUS, SImode, operands)
   && CONST_INT_P (operands[2])
   /* Check that operands[6] is operands[2] zero extended from
      SImode to DImode.  */
   && UINTVAL (operands[6]) == (UINTVAL (operands[2])
				& GET_MODE_MASK (SImode))"
  "adc{l}\t{%2, %0|%0, %2}"
  [(set_attr "type" "alu")
   (set_attr "use_carry" "1")
   (set_attr "pent_pair" "pu")
   (set_attr "mode" "SI")
   (set (attr "length_immediate")
     (if_then_else (match_test "IN_RANGE (INTVAL (operands[2]), -128, 127)")
       (const_string "1")
       (const_string "4")))])


More information about the Gcc-patches mailing list