Hi!

The bootstrap-ubsan bootstrap revealed a problem with the
{add,sub,mul}v<mode>4 patterns.  The problem is that they
accept CONST_INT operands (because the instructions do accept immediates),
but to model what the instruction actually does, we need to have both
the value of the operand itself and it's sign extended value to
2x wider mode, but say (sign_extend:DI (const_int 5)) in the pattern is
invalid RTL (found about this because e.g. num_sign_bit_copies returns
bogus return values on (sign_extend:DI (const_int 0)) ).
The following patch attempts to fix this by using two different define_insns
for each, one that accepts everything but VOIDmode operands (i.e. usually
register, memory, some SYMBOL_REFs/LABEL_REFs/CONSTs where we do have mode),
one that accepts only CONST_INTs.  Hopefully at least the combiner will
canonicalize the potential (sign_extend:DI (const_int N)) into
just (const_int N) and thus the *v<mode>4_1 insns would match (plus the
expander expands it that way too).

Bootstrapped/regtested on x86_64-linux and i686-linux, tested with
i686-linux --with-build-config=bootstrap-ubsan bootstrap.  Ok for trunk?

2014-03-25  Jakub Jelinek  <ja...@redhat.com>

        * config/i386/i386.md (addv<mode>4, subv<mode>4, mulv<mode>4): If
        operands[2] is CONST_INT, don't generate (sign_extend (const_int)).
        (*addv<mode>4, *subv<mode>4, *mulv<mode>4): Disallow CONST_INT_P
        operands[2].  Use We constraint instead of <i>.
        (*addv<mode>4_1, *subv<mode>4_1, *mulv<mode>4_1): New insns.
        * config/i386/constraints.md (We): New constraint.

--- gcc/config/i386/i386.md.jj  2014-03-19 08:14:37.000000000 +0100
+++ gcc/config/i386/i386.md     2014-03-24 15:03:24.899354663 +0100
@@ -5821,10 +5821,11 @@ (define_expand "addv<mode>4"
                   (eq:CCO (plus:<DWI>
                              (sign_extend:<DWI>
                                 (match_operand:SWI 1 "nonimmediate_operand"))
-                             (sign_extend:<DWI>
-                                (match_operand:SWI 2 "<general_operand>")))
+                             (match_dup 4))
                           (sign_extend:<DWI>
-                             (plus:SWI (match_dup 1) (match_dup 2)))))
+                             (plus:SWI (match_dup 1)
+                                       (match_operand:SWI 2
+                                          "<general_operand>")))))
              (set (match_operand:SWI 0 "register_operand")
                   (plus:SWI (match_dup 1) (match_dup 2)))])
    (set (pc) (if_then_else
@@ -5832,7 +5833,11 @@ (define_expand "addv<mode>4"
               (label_ref (match_operand 3))
               (pc)))]
   ""
-  "ix86_fixup_binary_operands_no_copy (PLUS, <MODE>mode, operands);")
+  "ix86_fixup_binary_operands_no_copy (PLUS, <MODE>mode, operands);
+   if (CONST_INT_P (operands[2]))
+     operands[4] = operands[2];
+   else
+     operands[4] = gen_rtx_SIGN_EXTEND (<DWI>mode, operands[2]);")
 
 (define_insn "*addv<mode>4"
   [(set (reg:CCO FLAGS_REG)
@@ -5840,16 +5845,42 @@ (define_insn "*addv<mode>4"
                   (sign_extend:<DWI>
                      (match_operand:SWI 1 "nonimmediate_operand" "%0,0"))
                   (sign_extend:<DWI>
-                     (match_operand:SWI 2 "<general_operand>" "<g>,<r><i>")))
+                     (match_operand:SWI 2 "<general_operand>" "<r>mWe,<r>We")))
                (sign_extend:<DWI>
                   (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)"
+  "ix86_binary_operator_ok (PLUS, <MODE>mode, operands)
+   && !CONST_INT_P (operands[2])"
   "add{<imodesuffix>}\t{%2, %0|%0, %2}"
   [(set_attr "type" "alu")
    (set_attr "mode" "<MODE>")])
 
+(define_insn "*addv<mode>4_1"
+  [(set (reg:CCO FLAGS_REG)
+       (eq:CCO (plus:<DWI>
+                  (sign_extend:<DWI>
+                     (match_operand:SWI 1 "nonimmediate_operand" "0"))
+                  (match_operand:<DWI> 3 "const_int_operand" "i"))
+               (sign_extend:<DWI>
+                  (plus:SWI (match_dup 1)
+                            (match_operand:SWI 2 "x86_64_immediate_operand"
+                                                 "<i>")))))
+   (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m")
+       (plus:SWI (match_dup 1) (match_dup 2)))]
+  "ix86_binary_operator_ok (PLUS, <MODE>mode, operands)
+   && CONST_INT_P (operands[2])
+   && INTVAL (operands[2]) == INTVAL (operands[3])"
+  "add{<imodesuffix>}\t{%2, %0|%0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "<MODE>")
+   (set (attr "length_immediate")
+       (cond [(match_test "IN_RANGE (INTVAL (operands[2]), -128, 127)")
+                 (const_string "1")
+              (match_test "<MODE_SIZE> == 8")
+                 (const_string "4")]
+             (const_string "<MODE_SIZE>")))])
+
 ;; The lea patterns for modes less than 32 bits need to be matched by
 ;; several insns converted to real lea by splitters.
 
@@ -6093,10 +6124,11 @@ (define_expand "subv<mode>4"
                   (eq:CCO (minus:<DWI>
                              (sign_extend:<DWI>
                                 (match_operand:SWI 1 "nonimmediate_operand"))
-                             (sign_extend:<DWI>
-                                (match_operand:SWI 2 "<general_operand>")))
+                             (match_dup 4))
                           (sign_extend:<DWI>
-                             (minus:SWI (match_dup 1) (match_dup 2)))))
+                             (minus:SWI (match_dup 1)
+                                        (match_operand:SWI 2
+                                           "<general_operand>")))))
              (set (match_operand:SWI 0 "register_operand")
                   (minus:SWI (match_dup 1) (match_dup 2)))])
    (set (pc) (if_then_else
@@ -6104,7 +6136,11 @@ (define_expand "subv<mode>4"
               (label_ref (match_operand 3))
               (pc)))]
   ""
-  "ix86_fixup_binary_operands_no_copy (MINUS, <MODE>mode, operands);")
+  "ix86_fixup_binary_operands_no_copy (MINUS, <MODE>mode, operands);
+   if (CONST_INT_P (operands[2]))
+     operands[4] = operands[2];
+   else
+     operands[4] = gen_rtx_SIGN_EXTEND (<DWI>mode, operands[2]);")
 
 (define_insn "*subv<mode>4"
   [(set (reg:CCO FLAGS_REG)
@@ -6112,16 +6148,42 @@ (define_insn "*subv<mode>4"
                   (sign_extend:<DWI>
                      (match_operand:SWI 1 "nonimmediate_operand" "0,0"))
                   (sign_extend:<DWI>
-                     (match_operand:SWI 2 "<general_operand>" "<r><i>,<r>m")))
+                     (match_operand:SWI 2 "<general_operand>" "<r>We,<r>m")))
                (sign_extend:<DWI>
                   (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)"
+  "ix86_binary_operator_ok (MINUS, <MODE>mode, operands)
+   && !CONST_INT_P (operands[2])"
   "sub{<imodesuffix>}\t{%2, %0|%0, %2}"
   [(set_attr "type" "alu")
    (set_attr "mode" "<MODE>")])
 
+(define_insn "*subv<mode>4_1"
+  [(set (reg:CCO FLAGS_REG)
+       (eq:CCO (minus:<DWI>
+                  (sign_extend:<DWI>
+                     (match_operand:SWI 1 "nonimmediate_operand" "0"))
+                  (match_operand:<DWI> 3 "const_int_operand" "i"))
+               (sign_extend:<DWI>
+                  (minus:SWI (match_dup 1)
+                             (match_operand:SWI 2 "x86_64_immediate_operand"
+                                                  "<i>")))))
+   (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m")
+       (minus:SWI (match_dup 1) (match_dup 2)))]
+  "ix86_binary_operator_ok (MINUS, <MODE>mode, operands)
+   && CONST_INT_P (operands[2])
+   && INTVAL (operands[2]) == INTVAL (operands[3])"
+  "sub{<imodesuffix>}\t{%2, %0|%0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "<MODE>")
+   (set (attr "length_immediate")
+       (cond [(match_test "IN_RANGE (INTVAL (operands[2]), -128, 127)")
+                 (const_string "1")
+              (match_test "<MODE_SIZE> == 8")
+                 (const_string "4")]
+             (const_string "<MODE_SIZE>")))])
+
 (define_insn "*sub<mode>_3"
   [(set (reg FLAGS_REG)
        (compare (match_operand:SWI 1 "nonimmediate_operand" "0,0")
@@ -6442,52 +6504,96 @@ (define_expand "mulv<mode>4"
                   (eq:CCO (mult:<DWI>
                              (sign_extend:<DWI>
                                 (match_operand:SWI48 1 "register_operand"))
-                             (sign_extend:<DWI>
-                                (match_operand:SWI48 2 "<general_operand>")))
+                             (match_dup 4))
                           (sign_extend:<DWI>
-                             (mult:SWI48 (match_dup 1) (match_dup 2)))))
+                             (mult:SWI48 (match_dup 1)
+                                         (match_operand:SWI48 2
+                                            "<general_operand>")))))
              (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)))])
+              (pc)))]
+  ""
+  "if (CONST_INT_P (operands[2]))
+     operands[4] = operands[2];
+   else
+     operands[4] = gen_rtx_SIGN_EXTEND (<DWI>mode, operands[2]);")
 
 (define_insn "*mulv<mode>4"
   [(set (reg:CCO FLAGS_REG)
        (eq:CCO (mult:<DWI>
                   (sign_extend:<DWI>
-                     (match_operand:SWI 1 "nonimmediate_operand" "%rm,rm,0"))
+                     (match_operand:SWI48 1 "nonimmediate_operand" "%rm,0"))
                   (sign_extend:<DWI>
-                     (match_operand:SWI 2 "<general_operand>" "K,<i>,mr")))
+                     (match_operand:SWI48 2 "<general_operand>" "We,mr")))
                (sign_extend:<DWI>
-                  (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]))"
+                  (mult:SWI48 (match_dup 1) (match_dup 2)))))
+   (set (match_operand:SWI48 0 "register_operand" "=r,r")
+       (mult:SWI48 (match_dup 1) (match_dup 2)))]
+  "!(MEM_P (operands[1]) && MEM_P (operands[2]))
+   && !CONST_INT_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 "prefix_0f" "0,1")
    (set (attr "athlon_decode")
        (cond [(eq_attr "cpu" "athlon")
                  (const_string "vector")
-              (eq_attr "alternative" "1")
+              (eq_attr "alternative" "0")
                  (const_string "vector")
-              (and (eq_attr "alternative" "2")
+              (and (eq_attr "alternative" "1")
                    (match_operand 1 "memory_operand"))
                  (const_string "vector")]
              (const_string "direct")))
    (set (attr "amdfam10_decode")
-       (cond [(and (eq_attr "alternative" "0,1")
+       (cond [(and (eq_attr "alternative" "1")
                    (match_operand 1 "memory_operand"))
                  (const_string "vector")]
              (const_string "direct")))
    (set_attr "bdver1_decode" "direct")
    (set_attr "mode" "<MODE>")])
 
+(define_insn "*mulv<mode>4_1"
+  [(set (reg:CCO FLAGS_REG)
+       (eq:CCO (mult:<DWI>
+                  (sign_extend:<DWI>
+                     (match_operand:SWI48 1 "nonimmediate_operand" "rm,rm"))
+                  (match_operand:<DWI> 3 "const_int_operand" "K,i"))
+               (sign_extend:<DWI>
+                  (mult:SWI48 (match_dup 1)
+                              (match_operand:SWI 2 "x86_64_immediate_operand"
+                                                   "K,<i>")))))
+   (set (match_operand:SWI48 0 "register_operand" "=r,r")
+       (mult:SWI48 (match_dup 1) (match_dup 2)))]
+  "!(MEM_P (operands[1]) && MEM_P (operands[2]))
+   && CONST_INT_P (operands[2])
+   && INTVAL (operands[2]) == INTVAL (operands[3])"
+  "@
+   imul{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}
+   imul{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}"
+  [(set_attr "type" "imul")
+   (set (attr "athlon_decode")
+       (cond [(eq_attr "cpu" "athlon")
+                 (const_string "vector")
+              (eq_attr "alternative" "1")
+                 (const_string "vector")]
+             (const_string "direct")))
+   (set (attr "amdfam10_decode")
+       (cond [(match_operand 1 "memory_operand")
+                 (const_string "vector")]
+             (const_string "direct")))
+   (set_attr "bdver1_decode" "direct")
+   (set_attr "mode" "<MODE>")
+   (set (attr "length_immediate")
+       (cond [(match_test "IN_RANGE (INTVAL (operands[2]), -128, 127)")
+                 (const_string "1")
+              (match_test "<MODE_SIZE> == 8")
+                 (const_string "4")]
+             (const_string "<MODE_SIZE>")))])
+
 (define_expand "<u>mul<mode><dwi>3"
   [(parallel [(set (match_operand:<DWI> 0 "register_operand")
                   (mult:<DWI>
--- gcc/config/i386/constraints.md.jj   2014-01-31 22:22:10.000000000 +0100
+++ gcc/config/i386/constraints.md      2014-03-24 15:01:08.831074223 +0100
@@ -220,6 +220,13 @@ (define_constraint "e"
 ;; We use W prefix to denote any number of
 ;; constant-or-symbol-reference constraints
 
+(define_constraint "We"
+  "32-bit signed integer constant, or a symbolic reference known
+   to fit that range (for sign-extending conversion operations that
+   require non-VOIDmode immediate operands)."
+  (and (match_operand 0 "x86_64_immediate_operand")
+       (match_test "GET_MODE (op) != VOIDmode")))
+
 (define_constraint "Wz"
   "32-bit unsigned integer constant, or a symbolic reference known
    to fit that range (for zero-extending conversion operations that

        Jakub

Reply via email to