| Message ID | 1461863430-22916-1-git-send-email-claziss@synopsys.com |
|---|---|
| State | New |
| Headers | show |
On 28/04/16 18:10, Claudiu Zissulescu wrote: > Please find the updated patch. > > Claudiu > > gcc/ > 2016-04-28 Claudiu Zissulescu <claziss@synopsys.com> > > * config/arc/arc.h (UNSIGNED_INT12, UNSIGNED_INT16): Define. > * config/arc/arc.md (umulhisi3): Use arc_short_operand predicate. > (umulhisi3_imm): Update predicates and constraint letters. > (umulhisi3_reg): Declare instruction as commutative. > * config/arc/constraints.md (U12, U16): New constraints. I'm not sure how to feel about this. U16 looks intuitive, but we have traditionally used U for memory constraints. And we use it for ARC for that purpose, too, even though with a compatible constraint length of 3. I suppose it's fine if you're sure we never want to have an addressing mode that's best described with "12" or "16", or some other number we might want for an unsigned integer. Otherwise, I'd suggest using a traditional integer letter. 'J' is free. > > (define_expand "umulhisi3" > [(set (match_operand:SI 0 "register_operand" "") > (mult:SI (zero_extend:SI (match_operand:HI 1 "register_operand" "")) > - (zero_extend:SI (match_operand:HI 2 "nonmemory_operand" ""))))] > + (zero_extend:SI (match_operand:HI 2 "arc_short_operand" ""))))] > "TARGET_MPYW" > "{ > if (CONSTANT_P (operands[2])) > { > - emit_insn (gen_umulhisi3_imm (operands[0], operands[1], operands[2])); > - DONE; > + emit_insn (gen_umulhisi3_imm (operands[0], operands[1], operands[2])); > + DONE; Why do you remove half of the indentation?
> > Otherwise, I'd suggest using a traditional integer letter. 'J' is free. Thanks for the suggestion, I will use 'J'. > Why do you remove half of the indentation? Unwanted reformatting, sorry for this, I will revert it. I have the feeling you are happy with my new patch. Is there anything to be added to it besides fixing the above issues? Thanks, Claudiu
On 28/04/16 21:31, Claudiu Zissulescu wrote: >> >> Otherwise, I'd suggest using a traditional integer letter. 'J' is free. > Thanks for the suggestion, I will use 'J'. > >> Why do you remove half of the indentation? > Unwanted reformatting, sorry for this, I will revert it. > > I have the feeling you are happy with my new patch. Is there anything > to be added to it besides fixing the above issues? No, otherwise it looks OK.
Committed r235623. Thanks, Claudiu > -----Original Message----- > From: Joern Wolfgang Rennecke [mailto:gnu@amylaar.uk] > Sent: Thursday, April 28, 2016 10:57 PM > To: Claudiu Zissulescu; Claudiu Zissulescu; gcc-patches@gcc.gnu.org > Cc: Francois.Bedard@synopsys.com; jeremy.bennett@embecosm.com > Subject: Re: [PATCH] [ARC] Fix unwanted match for sign extend 16-bit > constant. > > > > On 28/04/16 21:31, Claudiu Zissulescu wrote: > >> > >> Otherwise, I'd suggest using a traditional integer letter. 'J' is free. > > Thanks for the suggestion, I will use 'J'. > > > >> Why do you remove half of the indentation? > > Unwanted reformatting, sorry for this, I will revert it. > > > > I have the feeling you are happy with my new patch. Is there anything > > to be added to it besides fixing the above issues? > No, otherwise it looks OK.
diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h index 37c1afa..1b75099 100644 --- a/gcc/config/arc/arc.h +++ b/gcc/config/arc/arc.h @@ -795,6 +795,8 @@ extern enum reg_class arc_regno_reg_class[]; #define UNSIGNED_INT6(X) ((unsigned) (X) < 0x40) #define UNSIGNED_INT7(X) ((unsigned) (X) < 0x80) #define UNSIGNED_INT8(X) ((unsigned) (X) < 0x100) +#define UNSIGNED_INT12(X) ((unsigned) (X) < 0x800) +#define UNSIGNED_INT16(X) ((unsigned) (X) < 0x10000) #define IS_ONE(X) ((X) == 1) #define IS_ZERO(X) ((X) == 0) diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index 8ec0ce0..e0f74e4 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -1720,21 +1720,21 @@ (define_expand "umulhisi3" [(set (match_operand:SI 0 "register_operand" "") (mult:SI (zero_extend:SI (match_operand:HI 1 "register_operand" "")) - (zero_extend:SI (match_operand:HI 2 "nonmemory_operand" ""))))] + (zero_extend:SI (match_operand:HI 2 "arc_short_operand" ""))))] "TARGET_MPYW" "{ if (CONSTANT_P (operands[2])) { - emit_insn (gen_umulhisi3_imm (operands[0], operands[1], operands[2])); - DONE; + emit_insn (gen_umulhisi3_imm (operands[0], operands[1], operands[2])); + DONE; } }" ) (define_insn "umulhisi3_imm" - [(set (match_operand:SI 0 "register_operand" "=r, r,r, r, r") - (mult:SI (zero_extend:SI (match_operand:HI 1 "register_operand" " 0, r,0, 0, r")) - (match_operand:HI 2 "short_const_int_operand" " L, L,I,C16,C16")))] + [(set (match_operand:SI 0 "register_operand" "=r, r, r, r, r") + (mult:SI (zero_extend:SI (match_operand:HI 1 "register_operand" "%0, r, 0, 0, r")) + (match_operand:HI 2 "short_unsigned_const_operand" " L, L,U12,U16,U16")))] "TARGET_MPYW" "mpyuw%? %0,%1,%2" [(set_attr "length" "4,4,4,8,8") @@ -1746,7 +1746,7 @@ (define_insn "umulhisi3_reg" [(set (match_operand:SI 0 "register_operand" "=Rcqq, r, r") - (mult:SI (zero_extend:SI (match_operand:HI 1 "register_operand" " 0, 0, r")) + (mult:SI (zero_extend:SI (match_operand:HI 1 "register_operand" " %0, 0, r")) (zero_extend:SI (match_operand:HI 2 "register_operand" " Rcqq, r, r"))))] "TARGET_MPYW" "mpyuw%? %0,%1,%2" diff --git a/gcc/config/arc/constraints.md b/gcc/config/arc/constraints.md index 668b60a..cdf94ef 100644 --- a/gcc/config/arc/constraints.md +++ b/gcc/config/arc/constraints.md @@ -427,3 +427,14 @@ "A memory with only a base register" (match_operand 0 "mem_noofs_operand")) +(define_constraint "U12" + "@internal + An unsigned 12-bit integer constant." + (and (match_code "const_int") + (match_test "UNSIGNED_INT12 (ival)"))) + +(define_constraint "U16" + "@internal + An unsigned 16-bit integer constant" + (and (match_code "const_int") + (match_test "UNSIGNED_INT16 (ival)"))) diff --git a/gcc/config/arc/predicates.md b/gcc/config/arc/predicates.md index 3c657c6..9542b22 100644 --- a/gcc/config/arc/predicates.md +++ b/gcc/config/arc/predicates.md @@ -819,3 +819,11 @@ (define_predicate "double_register_operand" (ior (match_test "even_register_operand (op, mode)") (match_test "arc_double_register_operand (op, mode)"))) + +(define_predicate "short_unsigned_const_operand" + (and (match_code "const_int") + (match_test "satisfies_constraint_U16 (op)"))) + +(define_predicate "arc_short_operand" + (ior (match_test "register_operand (op, mode)") + (match_test "short_unsigned_const_operand (op, mode)"))) diff --git a/gcc/testsuite/gcc.target/arc/umulsihi3_z.c b/gcc/testsuite/gcc.target/arc/umulsihi3_z.c new file mode 100644 index 0000000..cf1c00d --- /dev/null +++ b/gcc/testsuite/gcc.target/arc/umulsihi3_z.c @@ -0,0 +1,23 @@ +/* Check if the optimizers are not removing the umulsihi3_imm + instruction. */ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-inline" } */ + +#include <stdint.h> + +static int32_t test (int16_t reg_val) +{ + int32_t x = (reg_val & 0xf) * 62500; + return x; +} + +int main (void) +{ + volatile int32_t x = 0xc172; + x = test (x); + + if (x != 0x0001e848) + __builtin_abort (); + return 0; +} +