* [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).