On Tue, Mar 25, 2014 at 09:12:14AM +0100, Uros Bizjak wrote:
> > 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?
>
> It looks to me that you will also need a corresponding predicate,
> similar to x86_64_zext_operand that rejects sign-extended VOIDmode
> operands where We constraint is used. That will also solve the problem
> with combiner, which will be prohibited from combining VOIDmode
> operands.
>
> Also, please use curly braces in multi-line preparation statements.
Like this? Or do you prefer a different name for the predicate?
And, is it ok to use just one predicate for both {QI,HI}mode and
{SI,DI}mode, or should I add separate predicates for "general_operand
where GET_MODE (op) != VOIDmode" and "x86_64_general_operand where
GET_MODE (op) != VOIDmode" and add a mode attribute for that?
If so, what would be your preferred names for the 2 predicates and for the
mode attribute?
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> and x86_64_sext_operand
predicate instead of <general_operand>.
(*addv<mode>4_1, *subv<mode>4_1, *mulv<mode>4_1): New insns.
* config/i386/constraints.md (We): New constraint.
* config/i386/predicates.md (x86_64_sext_operand): New predicate.
--- gcc/config/i386/i386.md.jj 2014-03-25 09:22:01.796149575 +0100
+++ gcc/config/i386/i386.md 2014-03-25 11:02:55.194817717 +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,13 @@ (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,7 +5847,8 @@ (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 "x86_64_sext_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")
@@ -5850,6 +5858,31 @@ (define_insn "*addv<mode>4"
[(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 +6126,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 +6138,13 @@ (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,7 +6152,8 @@ (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 "x86_64_sext_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>")
@@ -6122,6 +6163,31 @@ (define_insn "*subv<mode>4"
[(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 +6508,97 @@ (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 "x86_64_sext_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)))]
+ (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]))"
"@
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-03-25 09:22:01.848149274 +0100
+++ gcc/config/i386/constraints.md 2014-03-25 10:51:51.190345252 +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
--- gcc/config/i386/predicates.md.jj 2014-03-05 08:28:37.000000000 +0100
+++ gcc/config/i386/predicates.md 2014-03-25 11:00:22.762628436 +0100
@@ -338,6 +338,16 @@ (define_predicate "x86_64_general_operan
(match_operand 0 "x86_64_immediate_operand"))
(match_operand 0 "general_operand")))
+;; Return true if OP is non-VOIDmode general operand representable
+;; on x86_64. This predicate is used in sign-extending conversion
+;; operations that require non-VOIDmode immediate operands.
+(define_predicate "x86_64_sext_operand"
+ (and (match_test "GET_MODE (op) != VOIDmode")
+ (if_then_else
+ (match_test "GET_MODE (op) == SImode || GET_MODE (op) == DImode")
+ (match_operand 0 "x86_64_general_operand")
+ (match_operand 0 "general_operand"))))
+
;; Return true if OP is representable on x86_64 as zero-extended operand.
;; This predicate is used in zero-extending conversion operations that
;; require non-VOIDmode immediate operands.
Jakub
