| Message ID | 55C32B8A.7080007@arm.com |
|---|---|
| State | New |
| Headers | show |
> On Aug 6, 2015, at 11:40 AM, Renlin Li <renlin.li@arm.com> wrote: > > Hi all, > > This is a simple patch to add a new cmovdi_insn_uxtw rtx pattern to aarch64 backend. > > For the following simple test case: > > unsigned long > foo (unsigned int a, unsigned int b, unsigned int c) > { > return a ? b : c; > } > > With this new pattern, the new code-generation will be: > > cmp w0, wzr > csel x0, x2, x1, eq Your example Shows you have the wrong operand types to csel. In the aarch64 abi arguments don't need to be zero extended and your csel will not be zero extending the arguments. Note you should also use unsigned long long in the testcase so it is ilp32 and llp64l32 friendly. Thanks, Andrew > ret > > Without the path, the old code-generation is like this: > uxtw x1, w1 > cmp w0, wzr > uxtw x2, w2 > csel x0, x2, x1, eq > ret > > > aarch64-none-elf regression test Okay. Okay to commit? > > Regards, > Renlin Li > > gcc/ChangeLog: > > 2015-08-06 Renlin Li <renlin.li@arm.com> > > PR target/66776 > * config/aarch64/aarch64.md (cmovdi_insn_uxtw): New pattern. > > gcc/testsuite/ChangeLog: > > 2015-08-06 Renlin Li <renlin.li@arm.com> > > PR target/66776 > * gcc.target/aarch64/pr66776.c: New. > > > > <new-1.diff>
Hi Pinski, On 06/08/15 10:48, pinskia@gmail.com wrote: > > > >> On Aug 6, 2015, at 11:40 AM, Renlin Li <renlin.li@arm.com> wrote: >> >> Hi all, >> >> This is a simple patch to add a new cmovdi_insn_uxtw rtx pattern to aarch64 backend. >> >> For the following simple test case: >> >> unsigned long >> foo (unsigned int a, unsigned int b, unsigned int c) >> { >> return a ? b : c; >> } >> >> With this new pattern, the new code-generation will be: >> >> cmp w0, wzr >> csel x0, x2, x1, eq > Your example Shows you have the wrong operand types to csel. In the aarch64 abi arguments don't need to be zero extended and your csel will not be zero extending the arguments. Yes, I also just realized it. This patch is indeed not correct. Please ignore it. > > Note you should also use unsigned long long in the testcase so it is ilp32 and llp64l32 friendly. Yes, I will come up a new one. Thank you! Renlin > > Thanks, > Andrew > >
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index f264534..9c761f2 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -2887,6 +2887,27 @@ [(set_attr "type" "csel")] ) +(define_insn "*cmovdi_insn_uxtw" + [(set (match_operand:DI 0 "register_operand" "=r,r,r,r,r,r,r") + (if_then_else:DI + (match_operator 1 "aarch64_comparison_operator" + [(match_operand 2 "cc_register" "") (const_int 0)]) + (zero_extend:DI (match_operand:SI 3 "aarch64_reg_zero_or_m1_or_1" "rZ,rZ,UsM,rZ,Ui1,UsM,Ui1")) + (zero_extend:DI (match_operand:SI 4 "aarch64_reg_zero_or_m1_or_1" "rZ,UsM,rZ,Ui1,rZ,UsM,Ui1"))))] + "!((operands[3] == const1_rtx && operands[4] == constm1_rtx) + || (operands[3] == constm1_rtx && operands[4] == const1_rtx))" + ;; Final two alternatives should be unreachable, but included for completeness + "@ + csel\\t%x0, %x3, %x4, %m1 + csinv\\t%x0, %x3, xzr, %m1 + csinv\\t%x0, %x4, xzr, %M1 + csinc\\t%x0, %x3, xzr, %m1 + csinc\\t%x0, %x4, xzr, %M1 + mov\\t%x0, -1 + mov\\t%x0, 1" + [(set_attr "type" "csel")] +) + (define_insn "*cmov<mode>_insn" [(set (match_operand:GPF 0 "register_operand" "=w") (if_then_else:GPF diff --git a/gcc/testsuite/gcc.target/aarch64/pr66776.c b/gcc/testsuite/gcc.target/aarch64/pr66776.c new file mode 100644 index 0000000..695a474 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr66776.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 --save-temps" } */ + +unsigned long +foo (unsigned int a, unsigned int b, unsigned int c) +{ + return a ? b : c; +} + +/* { dg-final { scan-assembler-not "uxtw" } } */
Hi all, This is a simple patch to add a new cmovdi_insn_uxtw rtx pattern to aarch64 backend. For the following simple test case: unsigned long foo (unsigned int a, unsigned int b, unsigned int c) { return a ? b : c; } With this new pattern, the new code-generation will be: cmp w0, wzr csel x0, x2, x1, eq ret Without the path, the old code-generation is like this: uxtw x1, w1 cmp w0, wzr uxtw x2, w2 csel x0, x2, x1, eq ret aarch64-none-elf regression test Okay. Okay to commit? Regards, Renlin Li gcc/ChangeLog: 2015-08-06 Renlin Li <renlin.li@arm.com> PR target/66776 * config/aarch64/aarch64.md (cmovdi_insn_uxtw): New pattern. gcc/testsuite/ChangeLog: 2015-08-06 Renlin Li <renlin.li@arm.com> PR target/66776 * gcc.target/aarch64/pr66776.c: New.