* [PATCH v2] S/390: Allow LARL of literal pool entries
@ 2018-10-30 15:28 Ilya Leoshkevich
2018-10-30 17:59 ` Ulrich Weigand
0 siblings, 1 reply; 5+ messages in thread
From: Ilya Leoshkevich @ 2018-10-30 15:28 UTC (permalink / raw)
To: gcc-patches; +Cc: krebbel, rdapp, uweigand, Ilya Leoshkevich
Bootstrapped and regtested on s390x-redhat-linux.
Changes since v1:
* Removed unnecessary gen_rtx_CONST call.
* UNSPEC_LTREF are now omitted for all relatively addressed pool
entries, and not only those which occur in LARL-like patterns.
r265490 allowed the compiler to choose in a more flexible way whether to
use load or load-address-relative-long (LARL) instruction. When it
chose LARL for literal pool references, the latter ones were rewritten
by pass_s390_early_mach to use UNSPEC_LTREF, which assumes base register
usage, which in turn is not compatible with LARL. The end result was an
ICE because of unrecognizable insn.
UNSPEC_LTREF and friends are necessary in order to communicate the
dependency on the base register to pass_sched2. When LARL is used, no
base register is necessary, so in such cases the rewrite must be
avoided.
gcc/ChangeLog:
2018-10-26 Ilya Leoshkevich <iii@linux.ibm.com>
PR target/87762
* config/s390/predicates.md (larl_operand): Use
s390_symbol_larl_p () to reduce code duplication.
* config/s390/s390-protos.h (s390_symbol_larl_p): New function.
* config/s390/s390.c (s390_symbol_larl_p): New function.
(annotate_constant_pool_refs): Skip LARL operands.
(find_constant_pool_ref): Handle non-annotated literal pool
references, which are usable with LARL.
(replace_constant_pool_ref): Skip LARL operands.
---
gcc/config/s390/predicates.md | 9 ++-------
gcc/config/s390/s390-protos.h | 1 +
gcc/config/s390/s390.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/gcc/config/s390/predicates.md b/gcc/config/s390/predicates.md
index 98a824e77b7..0e431302479 100644
--- a/gcc/config/s390/predicates.md
+++ b/gcc/config/s390/predicates.md
@@ -151,9 +151,7 @@
if (GET_CODE (op) == LABEL_REF)
return true;
if (SYMBOL_REF_P (op))
- return (!SYMBOL_FLAG_NOTALIGN2_P (op)
- && SYMBOL_REF_TLS_MODEL (op) == 0
- && s390_rel_address_ok_p (op));
+ return s390_symbol_larl_p (op);
/* Everything else must have a CONST, so strip it. */
if (GET_CODE (op) != CONST)
@@ -176,10 +174,7 @@
if (GET_CODE (op) == LABEL_REF)
return true;
if (SYMBOL_REF_P (op))
- return (!SYMBOL_FLAG_NOTALIGN2_P (op)
- && SYMBOL_REF_TLS_MODEL (op) == 0
- && s390_rel_address_ok_p (op));
-
+ return s390_symbol_larl_p (op);
/* Now we must have a @GOTENT offset or @PLT stub
or an @INDNTPOFF TLS offset. */
diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h
index 96fa705f879..a5cd80fa446 100644
--- a/gcc/config/s390/s390-protos.h
+++ b/gcc/config/s390/s390-protos.h
@@ -157,6 +157,7 @@ extern void s390_indirect_branch_via_thunk (unsigned int regno,
rtx comparison_operator,
enum s390_indirect_branch_type type);
extern void s390_indirect_branch_via_inline_thunk (rtx execute_target);
+extern bool s390_symbol_larl_p (rtx);
#endif /* RTX_CODE */
/* s390-c.c routines */
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 29a829f48ea..bac215f647c 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -2816,6 +2816,16 @@ s390_decompose_constant_pool_ref (rtx *ref, rtx *disp, bool *is_ptr,
return true;
}
+/* Return true iff SYMBOL_REF X can be used with a LARL instruction. */
+
+bool
+s390_symbol_larl_p (rtx x)
+{
+ return (!SYMBOL_FLAG_NOTALIGN2_P (x)
+ && SYMBOL_REF_TLS_MODEL (x) == 0
+ && s390_rel_address_ok_p (x));
+}
+
/* Decompose a RTL expression ADDR for a memory address into
its components, returned in OUT.
@@ -8120,6 +8130,10 @@ annotate_constant_pool_refs (rtx *x)
int i, j;
const char *fmt;
+ /* Skip LARL operands, because they don't require a base register. */
+ if (larl_operand (*x, VOIDmode))
+ return;
+
gcc_assert (GET_CODE (*x) != SYMBOL_REF
|| !CONSTANT_POOL_ADDRESS_P (*x));
@@ -8223,6 +8237,18 @@ find_constant_pool_ref (rtx x, rtx *ref)
&& XINT (x, 1) == UNSPECV_POOL_ENTRY)
return;
+ if (SYMBOL_REF_P (x)
+ && CONSTANT_POOL_ADDRESS_P (x)
+ && s390_symbol_larl_p (x))
+ {
+ if (*ref == NULL_RTX)
+ *ref = x;
+ else
+ gcc_assert (*ref == x);
+
+ return;
+ }
+
gcc_assert (GET_CODE (x) != SYMBOL_REF
|| !CONSTANT_POOL_ADDRESS_P (x));
@@ -8264,6 +8290,10 @@ replace_constant_pool_ref (rtx *x, rtx ref, rtx offset)
int i, j;
const char *fmt;
+ /* Skip LARL operands, because they don't require a base register. */
+ if (larl_operand (*x, VOIDmode))
+ return;
+
gcc_assert (*x != ref);
if (GET_CODE (*x) == UNSPEC
--
2.19.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] S/390: Allow LARL of literal pool entries
2018-10-30 15:28 [PATCH v2] S/390: Allow LARL of literal pool entries Ilya Leoshkevich
@ 2018-10-30 17:59 ` Ulrich Weigand
2018-10-30 19:07 ` Ilya Leoshkevich
0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Weigand @ 2018-10-30 17:59 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: gcc-patches, krebbel, rdapp, Ilya Leoshkevich
Ilya Leoshkevich wrote:
> @@ -8223,6 +8237,18 @@ find_constant_pool_ref (rtx x, rtx *ref)
> && XINT (x, 1) == UNSPECV_POOL_ENTRY)
> return;
>
> + if (SYMBOL_REF_P (x)
> + && CONSTANT_POOL_ADDRESS_P (x)
> + && s390_symbol_larl_p (x))
> + {
> + if (*ref == NULL_RTX)
> + *ref = x;
> + else
> + gcc_assert (*ref == x);
> +
> + return;
> + }
This definitely looks wrong. If we haven't annotated the address,
it should *not* be found by find_constant_pool_ref, since we are
not going to replace it! That was the whole point of not annotating
it in the first place ...
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] S/390: Allow LARL of literal pool entries
2018-10-30 17:59 ` Ulrich Weigand
@ 2018-10-30 19:07 ` Ilya Leoshkevich
2018-10-31 10:28 ` Ulrich Weigand
0 siblings, 1 reply; 5+ messages in thread
From: Ilya Leoshkevich @ 2018-10-30 19:07 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: GCC Patches, krebbel, rdapp
> Am 30.10.2018 um 18:22 schrieb Ulrich Weigand <uweigand@de.ibm.com>:
>
> Ilya Leoshkevich wrote:
>
>> @@ -8223,6 +8237,18 @@ find_constant_pool_ref (rtx x, rtx *ref)
>> && XINT (x, 1) == UNSPECV_POOL_ENTRY)
>> return;
>>
>> + if (SYMBOL_REF_P (x)
>> + && CONSTANT_POOL_ADDRESS_P (x)
>> + && s390_symbol_larl_p (x))
>> + {
>> + if (*ref == NULL_RTX)
>> + *ref = x;
>> + else
>> + gcc_assert (*ref == x);
>> +
>> + return;
>> + }
>
> This definitely looks wrong. If we haven't annotated the address,
> it should *not* be found by find_constant_pool_ref, since we are
> not going to replace it! That was the whole point of not annotating
> it in the first place …
There are two use cases for find_constant_pool_ref (). One is indeed
replacing annotated references. The other (in s390_mainpool_start ()
and s390_chunkify_start ()) is creating pool entries. So I've decided
to let it find unannotated references for the second use case.
This impacts the first use case as well, that's why I have also changed
replace_constant_pool_ref () to ignore unannotated references.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] S/390: Allow LARL of literal pool entries
2018-10-30 19:07 ` Ilya Leoshkevich
@ 2018-10-31 10:28 ` Ulrich Weigand
2018-11-05 10:04 ` Ilya Leoshkevich
0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Weigand @ 2018-10-31 10:28 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: GCC Patches, krebbel, rdapp
Ilya Leoshkevich wrote:
> Am 30.10.2018 um 18:22 schrieb Ulrich Weigand <uweigand@de.ibm.com>:
> > This definitely looks wrong. If we haven't annotated the address,
> > it should *not* be found by find_constant_pool_ref, since we are
> > not going to replace it! That was the whole point of not annotating
> > it in the first place ...
>
> There are two use cases for find_constant_pool_ref (). One is indeed
> replacing annotated references. The other (in s390_mainpool_start ()
> and s390_chunkify_start ()) is creating pool entries. So I've decided
> to let it find unannotated references for the second use case.
OK, but if we access the constant via relative address, we don't need
to copy it into the back-end managed pool either; the relative address
can just refer the constant in the default pool maintained by the
middle end.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] S/390: Allow LARL of literal pool entries
2018-10-31 10:28 ` Ulrich Weigand
@ 2018-11-05 10:04 ` Ilya Leoshkevich
0 siblings, 0 replies; 5+ messages in thread
From: Ilya Leoshkevich @ 2018-11-05 10:04 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: GCC Patches, krebbel, rdapp
> Am 31.10.2018 um 10:59 schrieb Ulrich Weigand <uweigand@de.ibm.com>:
>
> Ilya Leoshkevich wrote:
>> Am 30.10.2018 um 18:22 schrieb Ulrich Weigand <uweigand@de.ibm.com>:
>>> This definitely looks wrong. If we haven't annotated the address,
>>> it should *not* be found by find_constant_pool_ref, since we are
>>> not going to replace it! That was the whole point of not annotating
>>> it in the first place ...
>>
>> There are two use cases for find_constant_pool_ref (). One is indeed
>> replacing annotated references. The other (in s390_mainpool_start ()
>> and s390_chunkify_start ()) is creating pool entries. So I've decided
>> to let it find unannotated references for the second use case.
>
> OK, but if we access the constant via relative address, we don't need
> to copy it into the back-end managed pool either; the relative address
> can just refer the constant in the default pool maintained by the
> middle end.
Wouldn’t that prevent constant merging in case the same constant is
used with both relative and base-register addressing?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-05 10:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 15:28 [PATCH v2] S/390: Allow LARL of literal pool entries Ilya Leoshkevich
2018-10-30 17:59 ` Ulrich Weigand
2018-10-30 19:07 ` Ilya Leoshkevich
2018-10-31 10:28 ` Ulrich Weigand
2018-11-05 10:04 ` Ilya Leoshkevich
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).