* [PATCH 0/2][GCC] Add one more pattern to RTL if-conversion @ 2016-05-23 13:53 Mikhail Maltsev 2016-05-23 13:58 ` [PATCH 1/2][GCC] Refactor noce_try_store_flag_constants Mikhail Maltsev 2016-05-23 14:00 ` [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion Mikhail Maltsev 0 siblings, 2 replies; 7+ messages in thread From: Mikhail Maltsev @ 2016-05-23 13:53 UTC (permalink / raw) To: gcc-patches, Richard Biener Hi all! Currently GCC generates rather bad code for the following test case: int test(int a) { return a % 2 != 0 ? 4 : 2; } The code looks like this: test: andl $1, %edi cmpl $1, %edi sbbl %eax, %eax andl $-2, %eax addl $4, %eax ret Clang seems to generate optimal code: test: andl $1, %edi leal 2(%rdi,%rdi), %eax retq After applying this series of 2 patches GCC generates: test: movl %edi, %eax andl $1, %eax leal 2(%rax,%rax), %eax ret -- Regards, Mikhail Maltsev ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2][GCC] Refactor noce_try_store_flag_constants 2016-05-23 13:53 [PATCH 0/2][GCC] Add one more pattern to RTL if-conversion Mikhail Maltsev @ 2016-05-23 13:58 ` Mikhail Maltsev 2016-06-02 9:28 ` Bernd Schmidt 2016-05-23 14:00 ` [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion Mikhail Maltsev 1 sibling, 1 reply; 7+ messages in thread From: Mikhail Maltsev @ 2016-05-23 13:58 UTC (permalink / raw) To: gcc-patches, Richard Biener [-- Attachment #1: Type: text/plain, Size: 614 bytes --] This patch refactors 'noce_try_store_flag_constants' a bit to make it easier to reason about. The function contains two series of conditions, and each branch of the second series corresponds to one or two branches of the first series. The patch introduces a new enumeration strategy_t instead and uses it to select the correct branch. Also, ISTM that the last 'else' branch is unreachable. Bootstrapped and regtested on x86_64-linux. OK for trunk? -- Regards, Mikhail Maltsev gcc/ChangeLog: 2016-05-23 Mikhail Maltsev <maltsevm@gmail.com> * ifcvt.c (noce_try_store_flag_constants): Refactor. [-- Attachment #2: 0001-Refactor-noce_try_store_flag_constants.patch --] [-- Type: application/x-patch, Size: 6078 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2][GCC] Refactor noce_try_store_flag_constants 2016-05-23 13:58 ` [PATCH 1/2][GCC] Refactor noce_try_store_flag_constants Mikhail Maltsev @ 2016-06-02 9:28 ` Bernd Schmidt 0 siblings, 0 replies; 7+ messages in thread From: Bernd Schmidt @ 2016-06-02 9:28 UTC (permalink / raw) To: Mikhail Maltsev, gcc-patches, Richard Biener On 05/23/2016 03:58 PM, Mikhail Maltsev wrote: > This patch refactors 'noce_try_store_flag_constants' a bit to make it easier to > reason about. > > The function contains two series of conditions, and each branch of the second > series corresponds to one or two branches of the first series. The patch > introduces a new enumeration strategy_t instead and uses it to select the > correct branch. Also, ISTM that the last 'else' branch is unreachable. > > Bootstrapped and regtested on x86_64-linux. OK for trunk? Please send patches as text/plain or text/x-patch attachments so they can be read and quoted. + bool can_reverse + = (reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN); Parens not needed anymore if the line is split this way. Otherwise OK. Maybe the test for common without ST_ADD_FLAG could be moved up a little to avoid unnecessary operations. Bernd ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion 2016-05-23 13:53 [PATCH 0/2][GCC] Add one more pattern to RTL if-conversion Mikhail Maltsev 2016-05-23 13:58 ` [PATCH 1/2][GCC] Refactor noce_try_store_flag_constants Mikhail Maltsev @ 2016-05-23 14:00 ` Mikhail Maltsev 2016-05-23 14:15 ` Kyrill Tkachov 1 sibling, 1 reply; 7+ messages in thread From: Mikhail Maltsev @ 2016-05-23 14:00 UTC (permalink / raw) To: gcc-patches, Richard Biener [-- Attachment #1: Type: text/plain, Size: 507 bytes --] This patch adds a new if-conversion pattern for the following case: if (test) x = A; else x = B; A and B are constants, abs(A - B) == 2^N, A != 0, B != 0 Bootstrapped and regtested on x86_64-linux. OK for trunk? -- Regards, Mikhail Maltsev gcc/testsuite/ChangeLog: 2016-05-23 Mikhail Maltsev <maltsevm@gmail.com> * gcc.dg/ifcvt-6.c: New test. gcc/ChangeLog: 2016-05-23 Mikhail Maltsev <maltsevm@gmail.com> * ifcvt.c (noce_try_store_flag_constants): Add new pattern. [-- Attachment #2: 0002-Add-ifcvt-pattern.patch --] [-- Type: application/x-patch, Size: 1943 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion 2016-05-23 14:00 ` [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion Mikhail Maltsev @ 2016-05-23 14:15 ` Kyrill Tkachov 2016-05-24 19:06 ` Mikhail Maltsev 0 siblings, 1 reply; 7+ messages in thread From: Kyrill Tkachov @ 2016-05-23 14:15 UTC (permalink / raw) To: Mikhail Maltsev, gcc-patches, Richard Biener Hi Mikhail, On 23/05/16 15:00, Mikhail Maltsev wrote: > This patch adds a new if-conversion pattern for the following case: > > if (test) x = A; else x = B; > > A and B are constants, abs(A - B) == 2^N, A != 0, B != 0 > > > Bootstrapped and regtested on x86_64-linux. OK for trunk? > @@ -1453,6 +1460,19 @@ noce_try_store_flag_constants (struct noce_if_info *if_info) gen_int_mode (ifalse, mode), if_info->x, 0, OPTAB_WIDEN); break; + case ST_SHIFT_ADD_FLAG: + { + /* if (test) x = 5; else x = 1; + => x = (test != 0) << 2 + 1; */ + HOST_WIDE_INT diff_log = exact_log2 (abs_hwi (diff)); + rtx diff_rtx + = expand_simple_binop (mode, ASHIFT, target, GEN_INT (diff_log), + if_info->x, 0, OPTAB_WIDEN); + target = expand_simple_binop (mode, (diff < 0) ? MINUS : PLUS, + gen_int_mode (ifalse, mode), diff_rtx, + if_info->x, 0, OPTAB_WIDEN); + break; + } expand_simple_binop may fail. I think you should add a check that diff_rtx is non-NULL and bail out early if it is. Kyrill ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion 2016-05-23 14:15 ` Kyrill Tkachov @ 2016-05-24 19:06 ` Mikhail Maltsev 2016-06-02 9:33 ` Bernd Schmidt 0 siblings, 1 reply; 7+ messages in thread From: Mikhail Maltsev @ 2016-05-24 19:06 UTC (permalink / raw) To: Kyrill Tkachov, gcc-patches, Richard Biener [-- Attachment #1: Type: text/plain, Size: 215 bytes --] On 05/23/2016 05:15 PM, Kyrill Tkachov wrote: > > expand_simple_binop may fail. I think you should add a check that diff_rtx is > non-NULL > and bail out early if it is. > Fixed. -- Regards, Mikhail Maltsev [-- Attachment #2: 0002-Add-ifcvt-pattern_v2.patch --] [-- Type: application/x-patch, Size: 1817 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion 2016-05-24 19:06 ` Mikhail Maltsev @ 2016-06-02 9:33 ` Bernd Schmidt 0 siblings, 0 replies; 7+ messages in thread From: Bernd Schmidt @ 2016-06-02 9:33 UTC (permalink / raw) To: Mikhail Maltsev, Kyrill Tkachov, gcc-patches, Richard Biener On 05/24/2016 07:21 PM, Mikhail Maltsev wrote: + target = expand_simple_binop (mode, (diff < 0) ? MINUS : PLUS, Parentheses around (diff < 0) aren't needed. Otherwise I think this is also OK. Bernd ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-02 9:33 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-23 13:53 [PATCH 0/2][GCC] Add one more pattern to RTL if-conversion Mikhail Maltsev 2016-05-23 13:58 ` [PATCH 1/2][GCC] Refactor noce_try_store_flag_constants Mikhail Maltsev 2016-06-02 9:28 ` Bernd Schmidt 2016-05-23 14:00 ` [PATCH 2/2][GCC] Add one more pattern to RTL if-conversion Mikhail Maltsev 2016-05-23 14:15 ` Kyrill Tkachov 2016-05-24 19:06 ` Mikhail Maltsev 2016-06-02 9:33 ` Bernd Schmidt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).