* [PATCH] recog.c: Fix RTX unsharing in change groups
@ 2011-03-04 12:05 Andreas Krebbel
2011-03-18 20:16 ` Eric Botcazou
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Krebbel @ 2011-03-04 12:05 UTC (permalink / raw)
To: gcc-patches
Hi,
we currently miss to do RTX unsharing in some cases when mixing
validate_change requests with and without rtx unsharing in the same
change group.
Debugging a setjmp testcase on s390 I've seen the following behaviour:
In an insn like A = B + C B is replaced with D in fold_rtx. Unsharing is
requested for D in this replacement (A = D + C). Later on fold_rtx
invokes canonicalize_change_group which decides to swap D and C
without requesting unsharing again (A = C + D). Since the unsharing
for a change group is delayed until confirm_change_group and is bound
to a location rather then the expression C is unshared instead of D
then.
The attached patch prevents that by doing RTX unsharing for all
subsequent changes after finding a single change where unsharing has
been explicitly requested.
Ok for mainline?
Bye,
-Andreas-
2011-03-04 Andreas Krebbel <Andreas.Krebbel@de.ibm.com>
* recog.c (confirm_change_group): Unshare all RTXs after finding
one in the group where unsharing has been requested.
Index: gcc/recog.c
===================================================================
*** gcc/recog.c.orig
--- gcc/recog.c
*************** confirm_change_group (void)
*** 454,465 ****
{
int i;
rtx last_object = NULL;
for (i = 0; i < num_changes; i++)
{
rtx object = changes[i].object;
! if (changes[i].unshare)
*changes[i].loc = copy_rtx (*changes[i].loc);
/* Avoid unnecessary rescanning when multiple changes to same instruction
--- 454,471 ----
{
int i;
rtx last_object = NULL;
+ bool unshare_p = false;
for (i = 0; i < num_changes; i++)
{
rtx object = changes[i].object;
! /* After doing one change which needs unsharing all subsequent
! changes need unsharing as well. A subsequent change might
! move parts of the object which have been requested to get
! unshared before without requesting unsharing itself. */
! unshare_p = unshare_p || changes[i].unshare;
! if (unshare_p)
*changes[i].loc = copy_rtx (*changes[i].loc);
/* Avoid unnecessary rescanning when multiple changes to same instruction
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] recog.c: Fix RTX unsharing in change groups
2011-03-04 12:05 [PATCH] recog.c: Fix RTX unsharing in change groups Andreas Krebbel
@ 2011-03-18 20:16 ` Eric Botcazou
2011-03-21 8:54 ` Andreas Krebbel
0 siblings, 1 reply; 4+ messages in thread
From: Eric Botcazou @ 2011-03-18 20:16 UTC (permalink / raw)
To: Andreas Krebbel; +Cc: gcc-patches
> The attached patch prevents that by doing RTX unsharing for all
> subsequent changes after finding a single change where unsharing has
> been explicitly requested.
This looks like a big hammer. Why not doing it in canonicalize_change_group,
i.e. calling validate_unshare_change instead of validate_change there?
--
Eric Botcazou
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] recog.c: Fix RTX unsharing in change groups
2011-03-18 20:16 ` Eric Botcazou
@ 2011-03-21 8:54 ` Andreas Krebbel
2011-03-21 10:10 ` Eric Botcazou
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Krebbel @ 2011-03-21 8:54 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On 03/18/2011 09:11 PM, Eric Botcazou wrote:
>> The attached patch prevents that by doing RTX unsharing for all
>> subsequent changes after finding a single change where unsharing has
>> been explicitly requested.
>
> This looks like a big hammer. Why not doing it in canonicalize_change_group,
> i.e. calling validate_unshare_change instead of validate_change there?
I think with the patch validate_(unshare)_change becomes easier to use. You can still
decide locally if you want unsharing or not. Without taking into account in which contexts
your function gets called. I consider this a big advantage.
Otherwise we would have to look at every validate_change invocation in order to check if
it might get called after validate_unshare_change. This sounds tedious and error-prone to
me. (and probably we would end up using validate_unshare_change everywhere)
-Andreas-
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] recog.c: Fix RTX unsharing in change groups
2011-03-21 8:54 ` Andreas Krebbel
@ 2011-03-21 10:10 ` Eric Botcazou
0 siblings, 0 replies; 4+ messages in thread
From: Eric Botcazou @ 2011-03-21 10:10 UTC (permalink / raw)
To: Andreas Krebbel; +Cc: gcc-patches
> I think with the patch validate_(unshare)_change becomes easier to use. You
> can still decide locally if you want unsharing or not. Without taking into
> account in which contexts your function gets called. I consider this a big
> advantage.
You're supposed to know what you're doing though and unsharing just because of
a single call to canonicalize_change_group in fold_rtx seems really overkill.
> Otherwise we would have to look at every validate_change invocation in
> order to check if it might get called after validate_unshare_change. This
> sounds tedious and error-prone to me. (and probably we would end up using
> validate_unshare_change everywhere)
Well, this essentially works fine as of this writing so it's a little hard to
buy this everything-is-probably-broken argument. canonicalize_change_group is
special since it's part of the recog.c API; it needs to support intertwined
calls with validate_change and validate_unshare_change. Once this is done,
it's reasonable to expect that the user of the API doesn't do crazy things.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-21 10:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-04 12:05 [PATCH] recog.c: Fix RTX unsharing in change groups Andreas Krebbel
2011-03-18 20:16 ` Eric Botcazou
2011-03-21 8:54 ` Andreas Krebbel
2011-03-21 10:10 ` Eric Botcazou
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).