On Wed, Dec 4, 2013 at 2:44 PM, Marek Polacek <pola...@redhat.com> wrote:
> And this is the i?86 specific part of -fsanitize=signed-integer-overflow,
> split out of the huge patch. It really is dependent on the generic
> parts, when commiting, I'll put both parts together.
>
> Uros, would you mind taking a look at this?
>
> Regtested/bootstrapped on x86_64-linux. Ok for trunk?
>
> 2013-12-04 Jakub Jelinek <ja...@redhat.com>
> Marek Polacek <pola...@redhat.com>
>
> * config/i386/i386.md (addv<mode>4, subv<mode>4, mulv<mode>4,
> negv<mode>3, negv<mode>3_1): Define expands.
> (*addv<mode>4, *subv<mode>4, *mulv<mode>4, *negv<mode>3): Define
> insns.
>
> --- gcc/config/i386/i386.md.mp 2013-12-04 12:15:33.508905947 +0100
> +++ gcc/config/i386/i386.md 2013-12-04 12:15:39.608929341 +0100
> @@ -6153,6 +6153,42 @@
> [(set_attr "type" "alu")
> (set_attr "mode" "QI")])
>
> +(define_mode_attr widerintmode [(QI "HI") (HI "SI") (SI "DI") (DI "TI")])
Please name this "widerint"" and put it just above existing DWI/dwi
mode attribute definitions. We will merge them together.
> +
> +;; Add with jump on overflow.
> +(define_expand "addv<mode>4"
> + [(parallel [(set (reg:CCO FLAGS_REG)
> + (eq:CCO (plus:<widerintmode>
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 1 "register_operand"))
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 2 "<general_operand>")))
> + (sign_extend:<widerintmode>
> + (plus:SWI (match_dup 1) (match_dup 2)))))
> + (set (match_operand:SWI 0 "register_operand")
> + (plus:SWI (match_dup 1) (match_dup 2)))])
> + (set (pc) (if_then_else
> + (eq (reg:CCO FLAGS_REG) (const_int 0))
> + (label_ref (match_operand 3))
> + (pc)))]
> + "")
Please use "nonimmediate_operand" for operand 1 and fixup input
operands with ix86_fixup_binary_operands_no_copy. Ideally, we could
use "nonimmediate_operand" also for operand 0, but in this case, we
would need to fixup output operand _after_ the PLUS pattern is emitted
- not worth, IMO.
Please also change sub expander below in this way.
> +(define_insn "*addv<mode>4"
> + [(set (reg:CCO FLAGS_REG)
> + (eq:CCO (plus:<widerintmode>
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 1 "nonimmediate_operand" "%0,0"))
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 2 "<general_operand>" "<g>,<r><i>")))
> + (sign_extend:<widerintmode>
> + (plus:SWI (match_dup 1) (match_dup 2)))))
> + (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>,<r>m")
> + (plus:SWI (match_dup 1) (match_dup 2)))]
> + "ix86_binary_operator_ok (PLUS, <MODE>mode, operands)"
> + "add{<imodesuffix>}\t{%2, %0|%0, %2}"
> + [(set_attr "type" "alu")
> + (set_attr "mode" "<MODE>")])
> +
> ;; The lea patterns for modes less than 32 bits need to be matched by
> ;; several insns converted to real lea by splitters.
>
> @@ -6390,6 +6426,40 @@
> [(set_attr "type" "alu")
> (set_attr "mode" "SI")])
>
> +;; Subtract with jump on overflow.
> +(define_expand "subv<mode>4"
> + [(parallel [(set (reg:CCO FLAGS_REG)
> + (eq:CCO (minus:<widerintmode>
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 1 "register_operand"))
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 2 "<general_operand>")))
> + (sign_extend:<widerintmode>
> + (minus:SWI (match_dup 1) (match_dup 2)))))
> + (set (match_operand:SWI 0 "register_operand")
> + (minus:SWI (match_dup 1) (match_dup 2)))])
> + (set (pc) (if_then_else
> + (eq (reg:CCO FLAGS_REG) (const_int 0))
> + (label_ref (match_operand 3))
> + (pc)))]
> + "")
> +
> +(define_insn "*subv<mode>4"
> + [(set (reg:CCO FLAGS_REG)
> + (eq:CCO (minus:<widerintmode>
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 1 "nonimmediate_operand" "0,0"))
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 2 "<general_operand>"
> "<r><i>,<r>m")))
> + (sign_extend:<widerintmode>
> + (minus:SWI (match_dup 1) (match_dup 2)))))
> + (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m,<r>")
> + (minus:SWI (match_dup 1) (match_dup 2)))]
> + "ix86_binary_operator_ok (MINUS, <MODE>mode, operands)"
> + "sub{<imodesuffix>}\t{%2, %0|%0, %2}"
> + [(set_attr "type" "alu")
> + (set_attr "mode" "<MODE>")])
> +
> (define_insn "*sub<mode>_3"
> [(set (reg FLAGS_REG)
> (compare (match_operand:SWI 1 "nonimmediate_operand" "0,0")
> @@ -6704,6 +6774,59 @@
> (set_attr "bdver1_decode" "direct")
> (set_attr "mode" "QI")])
>
> +;; Multiply with jump on overflow.
> +(define_expand "mulv<mode>4"
> + [(parallel [(set (reg:CCO FLAGS_REG)
> + (eq:CCO (mult:<widerintmode>
> + (sign_extend:<widerintmode>
> + (match_operand:SWI48 1 "register_operand"))
> + (sign_extend:<widerintmode>
> + (match_operand:SWI48 2 "<general_operand>")))
> + (sign_extend:<widerintmode>
> + (mult:SWI48 (match_dup 1) (match_dup 2)))))
> + (set (match_operand:SWI48 0 "register_operand")
> + (mult:SWI48 (match_dup 1) (match_dup 2)))])
> + (set (pc) (if_then_else
> + (eq (reg:CCO FLAGS_REG) (const_int 0))
> + (label_ref (match_operand 3))
> + (pc)))]
> + "")
Please remove empty quotes.
> +(define_insn "*mulv<mode>4"
> + [(set (reg:CCO FLAGS_REG)
> + (eq:CCO (mult:<widerintmode>
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 1 "nonimmediate_operand" "%rm,rm,0"))
> + (sign_extend:<widerintmode>
> + (match_operand:SWI 2 "<general_operand>" "K,<i>,mr")))
> + (sign_extend:<widerintmode>
> + (mult:SWI (match_dup 1) (match_dup 2)))))
> + (set (match_operand:SWI 0 "register_operand" "=r,r,r")
> + (mult:SWI (match_dup 1) (match_dup 2)))]
> + "!(MEM_P (operands[1]) && MEM_P (operands[2]))"
> + "@
> + imul{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}
> + imul{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}
> + imul{<imodesuffix>}\t{%2, %0|%0, %2}"
> + [(set_attr "type" "imul")
> + (set_attr "prefix_0f" "0,0,1")
> + (set (attr "athlon_decode")
> + (cond [(eq_attr "cpu" "athlon")
> + (const_string "vector")
> + (eq_attr "alternative" "1")
> + (const_string "vector")
> + (and (eq_attr "alternative" "2")
> + (match_operand 1 "memory_operand"))
> + (const_string "vector")]
> + (const_string "direct")))
> + (set (attr "amdfam10_decode")
> + (cond [(and (eq_attr "alternative" "0,1")
> + (match_operand 1 "memory_operand"))
> + (const_string "vector")]
> + (const_string "direct")))
> + (set_attr "bdver1_decode" "direct")
> + (set_attr "mode" "<MODE>")])
> +
> (define_expand "<u>mul<mode><dwi>3"
> [(parallel [(set (match_operand:<DWI> 0 "register_operand")
> (mult:<DWI>
> @@ -8617,6 +8740,49 @@
> [(set_attr "type" "negnot")
> (set_attr "mode" "SI")])
>
> +;; Negate with jump on overflow.
> +(define_expand "negv<mode>3"
> + [(parallel [(set (reg:CCO FLAGS_REG)
> + (ne:CCO (match_operand:SWI 1 "register_operand")
> + (const_int 0)))
> + (set (match_operand:SWI 0 "register_operand")
> + (neg:SWI (match_dup 1)))])
> + (set (pc) (if_then_else
> + (eq (reg:CCO FLAGS_REG) (const_int 0))
> + (label_ref (match_operand 2))
> + (pc)))]
> + ""
> +{
> + rtx minv = GEN_INT (HOST_WIDE_INT_M1U
> + << (GET_MODE_BITSIZE (<MODE>mode) - 1));
> + emit_insn (gen_negv<mode>3_1 (operands[0], operands[1], minv,
> operands[2]));
> + DONE;
> +})
No, please use
"operands[3] = GEN_INT (....);"
and use (match_dup 3) in the pattern. The pattern below is not needed then.
BTW: can we use
gen_int_mode (1 << (GET_MODE_BITSIZE (mode) - 1), mode)
instead?
> +
> +(define_expand "negv<mode>3_1"
> + [(parallel [(set (reg:CCO FLAGS_REG)
> + (ne:CCO (match_operand:SWI 1 "nonimmediate_operand")
> + (match_operand:SWI 2 "const_int_operand")))
> + (set (match_operand:SWI 0 "nonimmediate_operand")
> + (neg:SWI (match_dup 1)))])
> + (set (pc) (if_then_else
> + (eq (reg:CCO FLAGS_REG) (const_int 0))
> + (label_ref (match_operand 3))
> + (pc)))]
> + "")
> +
> +(define_insn "*negv<mode>3"
> + [(set (reg:CCO FLAGS_REG)
> + (ne:CCO (match_operand:SWI 1 "nonimmediate_operand" "0")
> + (match_operand:SWI 2 "const_int_operand")))
> + (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m")
> + (neg:SWI (match_dup 1)))]
> + "ix86_unary_operator_ok (NEG, <MODE>mode, operands)
> + && mode_signbit_p (<MODE>mode, operands[2])"
> + "neg{<imodesuffix>}\t%0"
> + [(set_attr "type" "negnot")
> + (set_attr "mode" "<MODE>")])
> +
> ;; Changing of sign for FP values is doable using integer unit too.
>
> (define_expand "<code><mode>2"
Please repost the patch with the fixes.
Thanks,
Uros.
