* [PATCH, PR66444] Handle -fipa-ra in reload_combine
@ 2015-06-08 12:05 Tom de Vries
2015-06-08 15:37 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2015-06-08 12:05 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
Hi,
this patch fixes PR66444, a problem with -fipa-ra in reload_combine.
The problem is that for the test-case, reload_combine combines these two
insns:
...
(insn 13 12 14 2 (parallel [
(set (reg/v/f:DI 37 r8 [orig:92 xD.1858 ] [92])
(plus:DI (reg:DI 37 r8 [96])
(reg:DI 0 ax [orig:95 D.1884 ] [95])))
(clobber (reg:CC 17 flags))
])
(expr_list:REG_EQUAL (plus:DI (reg:DI 0 ax [orig:95 D.1884 ] [95])
(const_int 962072674304 [0xe000000000]))
(nil)))
(insn 14 13 15 2 (set (reg:DI 5 di)
(reg/v/f:DI 37 r8 [orig:92 xD.1858 ] [92]))
(nil))
...
into this insn:
...
(insn 14 12 15 2 (set (reg:DI 5 di)
(plus:DI (reg:DI 37 r8 [96])
(reg:DI 0 ax [orig:95 D.1884 ] [95])))
(nil))
...
That removes the set of r8 by insn 13. And that set of r8 is used by
insn 16:
....
(call_insn 15 ...)
(insn 16 15 17 2 (set (reg:DI 5 di)
(reg/v/f:DI 37 r8 [orig:92 x ] [92])) test.c:33 85 {*movdi_internal}
(nil))
...
But reload_combine doesn't acknowledge that use, because it considers
that r8 is killed by call_insn 15.
The patch fixes the problem by using get_call_reg_set_usage to find out
that r8 is actually not killed by the call_insn.
Bootstrapped and reg-tested on x86_64 on top of trunk.
OK for trunk and gcc-5-branch?
Thanks,
- Tom
[-- Attachment #2: 0001-Handle-fipa-ra-in-reload_combine.patch --]
[-- Type: text/x-patch, Size: 1977 bytes --]
Handle -fipa-ra in reload_combine
2015-06-08 Tom de Vries <tom@codesourcery.com>
PR rtl-optimization/66444
* postreload.c (reload_combine): Use get_call_reg_set_usage instead of
call_used_regs.
* gcc.dg/pr66444.c: New test.
---
gcc/postreload.c | 5 +++-
gcc/testsuite/gcc.dg/pr66444.c | 58 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr66444.c
diff --git a/gcc/postreload.c b/gcc/postreload.c
index 7ecca15..1cc7b14 100644
--- a/gcc/postreload.c
+++ b/gcc/postreload.c
@@ -1352,9 +1352,12 @@ reload_combine (void)
if (CALL_P (insn))
{
rtx link;
+ HARD_REG_SET used_regs;
+
+ get_call_reg_set_usage (insn, &used_regs, call_used_reg_set);
for (r = 0; r < FIRST_PSEUDO_REGISTER; r++)
- if (call_used_regs[r])
+ if (TEST_HARD_REG_BIT (used_regs, r))
{
reg_state[r].use_index = RELOAD_COMBINE_MAX_USES;
reg_state[r].store_ruid = reload_combine_ruid;
diff --git a/gcc/testsuite/gcc.dg/pr66444.c b/gcc/testsuite/gcc.dg/pr66444.c
new file mode 100644
index 0000000..93a7644
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr66444.c
@@ -0,0 +1,58 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fipa-ra" } */
+
+extern void abort (void);
+
+int __attribute__((noinline, noclone))
+bar (void)
+{
+ return 1;
+}
+
+struct S
+{
+ unsigned long p, q, r;
+ void *v;
+};
+
+struct S *s1;
+struct S *s2;
+
+void __attribute__((noinline, noclone))
+fn2 (struct S *x)
+{
+ s2 = x;
+}
+
+__attribute__((noinline, noclone)) void *
+fn1 (struct S *x)
+{
+ /* Just a statement to make it a non-const function. */
+ s1 = x;
+
+ return (void *)0;
+}
+
+int __attribute__((noinline, noclone))
+baz (void)
+{
+ struct S *x = (struct S *) 0xe0000000U;
+
+ x += bar ();
+
+ fn1 (x);
+ fn2 (x);
+
+ return 0;
+}
+
+int
+main (void)
+{
+ baz ();
+
+ if (s2 != (((struct S *) 0xe0000000U) + 1))
+ abort ();
+
+ return 0;
+}
--
1.9.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, PR66444] Handle -fipa-ra in reload_combine
2015-06-08 12:05 [PATCH, PR66444] Handle -fipa-ra in reload_combine Tom de Vries
@ 2015-06-08 15:37 ` Jakub Jelinek
2015-06-08 17:47 ` Tom de Vries
2015-06-08 17:52 ` Tom de Vries
0 siblings, 2 replies; 4+ messages in thread
From: Jakub Jelinek @ 2015-06-08 15:37 UTC (permalink / raw)
To: Tom de Vries; +Cc: gcc-patches
On Mon, Jun 08, 2015 at 02:04:12PM +0200, Tom de Vries wrote:
> this patch fixes PR66444, a problem with -fipa-ra in reload_combine.
>
> The problem is that for the test-case, reload_combine combines these two
> insns:
Please work out with Vlad whether reload_cse_move2add doesn't need similar
fix (and check other spots too).
> 2015-06-08 Tom de Vries <tom@codesourcery.com>
>
> PR rtl-optimization/66444
> * postreload.c (reload_combine): Use get_call_reg_set_usage instead of
> call_used_regs.
LGTM.
> * gcc.dg/pr66444.c: New test.
> +int __attribute__((noinline, noclone))
> +baz (void)
> +{
> + struct S *x = (struct S *) 0xe0000000U;
I'm still afraid this will not really work on s390-linux (which has only
31-bit pointers) and will not work on 16-bit int targets either
(some have say 24-bit pointers etc., not really familiar with the embedded
world).
So, I'd suggest use a macro for the address, so you don't need to duplicate
it, and define it to say ((struct S *) 0x8000UL), if it reproduces
even with that change without your reload_combine fix.
Ok for trunk and 5.2 with that change.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, PR66444] Handle -fipa-ra in reload_combine
2015-06-08 15:37 ` Jakub Jelinek
@ 2015-06-08 17:47 ` Tom de Vries
2015-06-08 17:52 ` Tom de Vries
1 sibling, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2015-06-08 17:47 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]
On 08/06/15 17:31, Jakub Jelinek wrote:
> On Mon, Jun 08, 2015 at 02:04:12PM +0200, Tom de Vries wrote:
>> this patch fixes PR66444, a problem with -fipa-ra in reload_combine.
>>
>> The problem is that for the test-case, reload_combine combines these two
>> insns:
>
> Please work out with Vlad whether reload_cse_move2add doesn't need similar
> fix (and check other spots too).
>
>> 2015-06-08 Tom de Vries <tom@codesourcery.com>
>>
>> PR rtl-optimization/66444
>> * postreload.c (reload_combine): Use get_call_reg_set_usage instead of
>> call_used_regs.
>
> LGTM.
>
>> * gcc.dg/pr66444.c: New test.
>
>> +int __attribute__((noinline, noclone))
>> +baz (void)
>> +{
>> + struct S *x = (struct S *) 0xe0000000U;
>
> I'm still afraid this will not really work on s390-linux (which has only
> 31-bit pointers) and will not work on 16-bit int targets either
> (some have say 24-bit pointers etc., not really familiar with the embedded
> world).
> So, I'd suggest use a macro for the address, so you don't need to duplicate
> it,
Used a macro CONST_PTR.
> and define it to say ((struct S *) 0x8000UL), if it reproduces
> even with that change without your reload_combine fix.
Unfortunately, it didn't. So I used __SIZEOF_POINTER__ to produce a
valid constant pointer for the 16-31 bit pointer-size cases, while still
triggering the original problem for x86_64 -m64 case using a larger pointer.
Furthermore, I used __SIZEOF_POINTER__ to ensured a valid pointer
constant suffix.
>
> Ok for trunk and 5.2 with that change.
Committed to trunk and backported gcc-5-branch as attached.
Thanks,
- Tom
[-- Attachment #2: 0001-Handle-fipa-ra-in-reload_combine.patch --]
[-- Type: text/x-patch, Size: 2733 bytes --]
Handle -fipa-ra in reload_combine
2015-06-08 Tom de Vries <tom@codesourcery.com>
PR rtl-optimization/66444
* postreload.c (reload_combine): Use get_call_reg_set_usage instead of
call_used_regs.
* gcc.dg/pr66444.c: New test.
---
gcc/postreload.c | 5 ++-
gcc/testsuite/gcc.dg/pr66444.c | 79 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr66444.c
diff --git a/gcc/postreload.c b/gcc/postreload.c
index 7ecca15..1cc7b14 100644
--- a/gcc/postreload.c
+++ b/gcc/postreload.c
@@ -1352,9 +1352,12 @@ reload_combine (void)
if (CALL_P (insn))
{
rtx link;
+ HARD_REG_SET used_regs;
+
+ get_call_reg_set_usage (insn, &used_regs, call_used_reg_set);
for (r = 0; r < FIRST_PSEUDO_REGISTER; r++)
- if (call_used_regs[r])
+ if (TEST_HARD_REG_BIT (used_regs, r))
{
reg_state[r].use_index = RELOAD_COMBINE_MAX_USES;
reg_state[r].store_ruid = reload_combine_ruid;
diff --git a/gcc/testsuite/gcc.dg/pr66444.c b/gcc/testsuite/gcc.dg/pr66444.c
new file mode 100644
index 0000000..3f92a5c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr66444.c
@@ -0,0 +1,79 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fipa-ra" } */
+
+extern void abort (void);
+
+#if (__SIZEOF_LONG_LONG__ == __SIZEOF_POINTER__)
+#define ADD_SUFFIX(a) a ## ULL
+#elif (__SIZEOF_LONG__ == __SIZEOF_POINTER__)
+#define ADD_SUFFIX(a) a ## UL
+#elif (__SIZEOF_INT__ == __SIZEOF_POINTER__)
+#define ADD_SUFFIX(a) a ## U
+#else
+#error Add target support here
+#endif
+
+#if __SIZEOF_POINTER__ <= 4
+/* Use a 16 bit pointer to have a valid pointer for 16-bit to 31-bit pointer
+ architectures. Using sizeof, we cannot distinguish between 31-bit and 32-bit
+ pointer types, so we also handle the 32-bit pointer type case here. */
+#define CONST_PTR ADD_SUFFIX (0x800)
+#else
+/* For x86_64 -m64, the problem reproduces with this 32-bit CONST_PTR, but not
+ with a 2-power below it. */
+#define CONST_PTR ADD_SUFFIX (0x80000000)
+#endif
+
+int __attribute__((noinline, noclone))
+bar (void)
+{
+ return 1;
+}
+
+struct S
+{
+ unsigned long p, q, r;
+ void *v;
+};
+
+struct S *s1;
+struct S *s2;
+
+void __attribute__((noinline, noclone))
+fn2 (struct S *x)
+{
+ s2 = x;
+}
+
+__attribute__((noinline, noclone)) void *
+fn1 (struct S *x)
+{
+ /* Just a statement to make it a non-const function. */
+ s1 = x;
+
+ return (void *)0;
+}
+
+int __attribute__((noinline, noclone))
+baz (void)
+{
+ struct S *x = (struct S *) CONST_PTR;
+
+ x += bar ();
+
+ fn1 (x);
+ fn2 (x);
+
+ return 0;
+}
+
+int
+main (void)
+{
+ baz ();
+
+ if (s2 != (((struct S *) CONST_PTR) + 1))
+ abort ();
+
+ return 0;
+}
--
1.9.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, PR66444] Handle -fipa-ra in reload_combine
2015-06-08 15:37 ` Jakub Jelinek
2015-06-08 17:47 ` Tom de Vries
@ 2015-06-08 17:52 ` Tom de Vries
1 sibling, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2015-06-08 17:52 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 08/06/15 17:31, Jakub Jelinek wrote:
> On Mon, Jun 08, 2015 at 02:04:12PM +0200, Tom de Vries wrote:
>> >this patch fixes PR66444, a problem with -fipa-ra in reload_combine.
>> >
>> >The problem is that for the test-case, reload_combine combines these two
>> >insns:
> Please work out with Vlad whether reload_cse_move2add doesn't need similar
> fix (and check other spots too).
>
Filed PR66463 - review uses of call_used_regs and regs_invalidated_by_call.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-06-08 17:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-08 12:05 [PATCH, PR66444] Handle -fipa-ra in reload_combine Tom de Vries
2015-06-08 15:37 ` Jakub Jelinek
2015-06-08 17:47 ` Tom de Vries
2015-06-08 17:52 ` Tom de Vries
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).