* [PATCH] s390: Change costs for load on condition. @ 2022-01-20 10:10 Robin Dapp 2022-01-21 8:30 ` Andreas Krebbel 0 siblings, 1 reply; 3+ messages in thread From: Robin Dapp @ 2022-01-20 10:10 UTC (permalink / raw) To: GCC Patches, Andreas Krebbel [-- Attachment #1: Type: text/plain, Size: 740 bytes --] Hi, this patch is a follow-up patch to the recent ifcvt changes. It increased costs for a load on condition to 6. This ensures that we if-convert sequences of three regular instructions (of cost 4) e.g. a compare and two SETs into two loads on condition (of cost 6). With a cost of 5, four-insn sequences (three SETs) would also be if-converted. The adjustment to the mov[qi/si]cc expander makes sure we if-convert a QImode/bool. Before, combine would create a paradoxical subreg itself but need an additional insn. Bootstrapped and regtested on s390x. Is it OK? Regards Robin -- gcc/ChangeLog: * config/s390/s390.cc (s390_rtx_costs): Increase costs for load on condition. * config/s390/s390.md: Change mov[qi/si]cc expander. [-- Attachment #2: s390-ifcvt-qisi.patch --] [-- Type: text/x-patch, Size: 1244 bytes --] commit b246f96c2a2813d0e509e7916744cda07cc5131c Author: Robin Dapp <rdapp@linux.ibm.com> Date: Fri Jun 18 10:51:22 2021 +0200 s390: Increase costs for load on condition and change movqicc expander. diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc index 43c5c72554a..f2e4474df99 100644 --- a/gcc/config/s390/s390.cc +++ b/gcc/config/s390/s390.cc @@ -3636,7 +3636,7 @@ s390_rtx_costs (rtx x, machine_mode mode, int outer_code, /* It is going to be a load/store on condition. Make it slightly more expensive than a normal load. */ - *total = COSTS_N_INSNS (1) + 1; + *total = COSTS_N_INSNS (1) + 2; rtx dst = SET_DEST (x); rtx then = XEXP (SET_SRC (x), 1); diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md index e3ccbac58c0..5eee8e86b42 100644 --- a/gcc/config/s390/s390.md +++ b/gcc/config/s390/s390.md @@ -7003,9 +7003,9 @@ if (!CONSTANT_P (els)) els = simplify_gen_subreg (E_SImode, els, <MODE>mode, 0); - rtx tmp_target = gen_reg_rtx (E_SImode); + rtx tmp_target = simplify_gen_subreg (E_SImode, operands[0], <MODE>mode, 0); + emit_insn (gen_movsicc (tmp_target, operands[1], then, els)); - emit_move_insn (operands[0], gen_lowpart (<MODE>mode, tmp_target)); DONE; }) ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] s390: Change costs for load on condition. 2022-01-20 10:10 [PATCH] s390: Change costs for load on condition Robin Dapp @ 2022-01-21 8:30 ` Andreas Krebbel 2022-02-08 14:35 ` Robin Dapp 0 siblings, 1 reply; 3+ messages in thread From: Andreas Krebbel @ 2022-01-21 8:30 UTC (permalink / raw) To: Robin Dapp, GCC Patches On 1/20/22 11:10, Robin Dapp wrote: > Hi, > > this patch is a follow-up patch to the recent ifcvt changes. It > increased costs for a load on condition to 6. This ensures that we > if-convert sequences of three regular instructions (of cost 4) e.g. a > compare and two SETs into two loads on condition (of cost 6). With a > cost of 5, four-insn sequences (three SETs) would also be if-converted. > > The adjustment to the mov[qi/si]cc expander makes sure we if-convert a > QImode/bool. Before, combine would create a paradoxical subreg itself > but need an additional insn. > > Bootstrapped and regtested on s390x. > > Is it OK? > > Regards > Robin > > -- > > gcc/ChangeLog: > > * config/s390/s390.cc (s390_rtx_costs): Increase costs for load > on condition. > * config/s390/s390.md: Change mov[qi/si]cc expander. Could you please add two tests for the sequences which are improved here. Just to make sure we get aware once it breaks again. Patch is ok. Thanks! Andreas ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] s390: Change costs for load on condition. 2022-01-21 8:30 ` Andreas Krebbel @ 2022-02-08 14:35 ` Robin Dapp 0 siblings, 0 replies; 3+ messages in thread From: Robin Dapp @ 2022-02-08 14:35 UTC (permalink / raw) To: Andreas Krebbel, GCC Patches [-- Attachment #1: Type: text/plain, Size: 86 bytes --] > Patch is ok. Thanks! As discussed off-list, committed as attached. Regards Robin [-- Attachment #2: bla.diff --] [-- Type: text/x-patch, Size: 3460 bytes --] commit 1e3185e714e877b2b4d14ade0865322f71a8cbf6 Author: Robin Dapp <rdapp@linux.ibm.com> Date: Tue Feb 8 14:56:29 2022 +0100 s390: Increase costs for load on condition and change movqicc expander. This patch changes the costs for a load on condition from 5 to 6 in order to ensure that we only if-convert two and not three or more SETS like if (cond) { a = b; c = d; e = f; } In the movqicc expander we emit a paradoxical subreg directly that combine would otherwise try to create by using a non-optimal sequence (which would be too expensive). Also, fix two oversights in ifcvt testcases. gcc/ChangeLog: * config/s390/s390.cc (s390_rtx_costs): Increase costs for load on condition. * config/s390/s390.md: Use paradoxical subreg. gcc/testsuite/ChangeLog: * gcc.target/s390/ifcvt-two-insns-int.c: Fix array size. * gcc.target/s390/ifcvt-two-insns-long.c: Dito. diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc index c6cfe41ad7b..d2af6d8813d 100644 --- a/gcc/config/s390/s390.cc +++ b/gcc/config/s390/s390.cc @@ -3636,7 +3636,7 @@ s390_rtx_costs (rtx x, machine_mode mode, int outer_code, /* It is going to be a load/store on condition. Make it slightly more expensive than a normal load. */ - *total = COSTS_N_INSNS (1) + 1; + *total = COSTS_N_INSNS (1) + 2; rtx dst = SET_DEST (x); rtx then = XEXP (SET_SRC (x), 1); diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md index e3ccbac58c0..5eee8e86b42 100644 --- a/gcc/config/s390/s390.md +++ b/gcc/config/s390/s390.md @@ -7003,9 +7003,9 @@ if (!CONSTANT_P (els)) els = simplify_gen_subreg (E_SImode, els, <MODE>mode, 0); - rtx tmp_target = gen_reg_rtx (E_SImode); + rtx tmp_target = simplify_gen_subreg (E_SImode, operands[0], <MODE>mode, 0); + emit_insn (gen_movsicc (tmp_target, operands[1], then, els)); - emit_move_insn (operands[0], gen_lowpart (<MODE>mode, tmp_target)); DONE; }) diff --git a/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-int.c b/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-int.c index 1ced7107de0..031cc433f56 100644 --- a/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-int.c +++ b/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-int.c @@ -5,7 +5,6 @@ /* { dg-final { scan-assembler "lochinle\t%r.?,1" } } */ /* { dg-final { scan-assembler "locrnle\t.*" } } */ -#include <stdbool.h> #include <limits.h> #include <stdio.h> #include <assert.h> @@ -33,7 +32,7 @@ int main() { int a[] = {2, 1, -13, INT_MAX, INT_MIN, 0}; - int res = foo (a, sizeof (a)); + int res = foo (a, sizeof (a) / sizeof (a[0])); assert (res == (INT_MIN + 1)); } diff --git a/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-long.c b/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-long.c index 5a1173fc6ef..cd04d2ad33e 100644 --- a/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-long.c +++ b/gcc/testsuite/gcc.target/s390/ifcvt-two-insns-long.c @@ -5,7 +5,6 @@ /* { dg-final { scan-assembler "locghinle\t%r.?,1" } } */ /* { dg-final { scan-assembler "locgrnle\t.*" } } */ -#include <stdbool.h> #include <limits.h> #include <stdio.h> #include <assert.h> @@ -33,7 +32,7 @@ int main() { long a[] = {2, 1, -13, LONG_MAX, LONG_MIN, 0}; - long res = foo (a, sizeof (a)); + long res = foo (a, sizeof (a) / sizeof (a[0])); assert (res == (LONG_MIN + 1)); } ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-08 14:35 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-20 10:10 [PATCH] s390: Change costs for load on condition Robin Dapp 2022-01-21 8:30 ` Andreas Krebbel 2022-02-08 14:35 ` Robin Dapp
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).