Fix PR49154-related SEGV in IRA. And ping.

Fix PR49154-related SEGV in IRA. And ping.

[Hans-Peter Nilsson]

First, a ping for
<http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00136.html>;
updated regclass documentation.  Ok?

Second, I updated the CRIS port to fit the proposed
documentation update (adding a class as the patch you sent, but
more complete), with regtest results clean for revisions before
the revision where build started failing.  But, after that
revision, I get a SEGV; details added to
<http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49154>.  At first
I thought I messed up the regclass description but it appears to
be just an obvious miss, fixed below.  If it isn't, I think
setup_regno_cost_classes_by_mode should have a comment as to who
else should set up regno_cost_classes[regno] and that "who else"
needs to be fixed to do the setup.  Odd that this bug didn't
trig before or for any of the targets on which you tested.
Maybe that makes cris-elf qualify for the set of targets you
test IRA changes on?

By the way, can we do a s/_aclass/_class/ in applicable files,
for example ira-costs.c?  Someone appears to have done a botched
word-replace of class to aclass; besides the identifier "class"
it changed occurrences within identifiers too (at least those
after a _) so we have e.g. cost_classes_aclass_cache and
setup_regno_cost_classes_by_aclass vs. cost_classes_mode_cache
and setup_regno_cost_classes_by_mode.

Tested on cris-elf; together with the mentioned update it
restores build with results consistent with those before the
breakage.

Ok to commit?

gcc:
	PR rtl-optimization/49154
	* ira-costs.c (setup_regno_cost_classes_by_mode): If there
	already is a matching slot in the hashtable, assign it to
	classes_ptr.

diff --git gcc/ira-costs.c gcc/ira-costs.c
index f517386..a22bb15 100644
--- gcc/ira-costs.c
+++ gcc/ira-costs.c
@@ -299,6 +299,8 @@ setup_regno_cost_classes_by_mode (int regno, enum machine_mode mode)
 	  classes_ptr = setup_cost_classes (&classes);
 	  *slot = classes_ptr;
 	}
+      else
+	classes_ptr = *slot;
       cost_classes_mode_cache[mode] = (cost_classes_t) *slot;
     }
   regno_cost_classes[regno] = classes_ptr;

brgds, H-P

Re: Fix PR49154-related SEGV in IRA. And ping.

[Vladimir Makarov]

On 06/08/2011 10:27 PM, Hans-Peter Nilsson wrote:
> First, a ping for
> <http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00136.html>;
> updated regclass documentation.  Ok?
>

Sorry for the delay.  I actually thought about a better formulation for 
some time and forgot about this because I had to work on other urgent 
things.

I'd not say that is the best description of the requirement but I can 
not formulate the better one which is not too complicated.  So it is 
better to have something close than nothing.  It is ok for me.

> Second, I updated the CRIS port to fit the proposed
> documentation update (adding a class as the patch you sent, but
> more complete), with regtest results clean for revisions before
> the revision where build started failing.  But, after that
> revision, I get a SEGV; details added to
> <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49154>.  At first
> I thought I messed up the regclass description but it appears to
> be just an obvious miss, fixed below.  If it isn't, I think
> setup_regno_cost_classes_by_mode should have a comment as to who
> else should set up regno_cost_classes[regno] and that "who else"
> needs to be fixed to do the setup.  Odd that this bug didn't
> trig before or for any of the targets on which you tested.
> Maybe that makes cris-elf qualify for the set of targets you
> test IRA changes on?
>
> By the way, can we do a s/_aclass/_class/ in applicable files,
> for example ira-costs.c?  Someone appears to have done a botched
> word-replace of class to aclass; besides the identifier "class"
> it changed occurrences within identifiers too (at least those
> after a _) so we have e.g. cost_classes_aclass_cache and
> setup_regno_cost_classes_by_aclass vs. cost_classes_mode_cache
> and setup_regno_cost_classes_by_mode.
>
> Tested on cris-elf; together with the mentioned update it
> restores build with results consistent with those before the
> breakage.
>
> Ok to commit?
>

Yes.  Thanks for fixing this.

> gcc:
> 	PR rtl-optimization/49154
> 	* ira-costs.c (setup_regno_cost_classes_by_mode): If there
> 	already is a matching slot in the hashtable, assign it to
> 	classes_ptr.
>
> diff --git gcc/ira-costs.c gcc/ira-costs.c
> index f517386..a22bb15 100644
> --- gcc/ira-costs.c
> +++ gcc/ira-costs.c
> @@ -299,6 +299,8 @@ setup_regno_cost_classes_by_mode (int regno, enum machine_mode mode)
>   	  classes_ptr = setup_cost_classes (&classes);
>   	  *slot = classes_ptr;
>   	}
> +      else
> +	classes_ptr = *slot;
>         cost_classes_mode_cache[mode] = (cost_classes_t) *slot;
>       }
>     regno_cost_classes[regno] = classes_ptr;
>
> brgds, H-P

Re: Fix PR49154-related SEGV in IRA. And ping.

[H.J. Lu]

On Wed, Jun 8, 2011 at 7:27 PM, Hans-Peter Nilsson <hp@bitrange.com> wrote:
> First, a ping for
> <http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00136.html>;
> updated regclass documentation.  Ok?
>
> Second, I updated the CRIS port to fit the proposed
> documentation update (adding a class as the patch you sent, but
> more complete), with regtest results clean for revisions before
> the revision where build started failing.  But, after that
> revision, I get a SEGV; details added to
> <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49154>.  At first
> I thought I messed up the regclass description but it appears to
> be just an obvious miss, fixed below.  If it isn't, I think
> setup_regno_cost_classes_by_mode should have a comment as to who
> else should set up regno_cost_classes[regno] and that "who else"
> needs to be fixed to do the setup.  Odd that this bug didn't
> trig before or for any of the targets on which you tested.
> Maybe that makes cris-elf qualify for the set of targets you
> test IRA changes on?
>
> By the way, can we do a s/_aclass/_class/ in applicable files,
> for example ira-costs.c?  Someone appears to have done a botched
> word-replace of class to aclass; besides the identifier "class"
> it changed occurrences within identifiers too (at least those
> after a _) so we have e.g. cost_classes_aclass_cache and
> setup_regno_cost_classes_by_aclass vs. cost_classes_mode_cache
> and setup_regno_cost_classes_by_mode.
>
> Tested on cris-elf; together with the mentioned update it
> restores build with results consistent with those before the
> breakage.
>
> Ok to commit?
>
> gcc:
>        PR rtl-optimization/49154
>        * ira-costs.c (setup_regno_cost_classes_by_mode): If there
>        already is a matching slot in the hashtable, assign it to
>        classes_ptr.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49354

-- 
H.J.

Re: Fix PR49154-related SEGV in IRA. And ping.

[Hans-Peter Nilsson]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1203 bytes --]

On Thu, 9 Jun 2011, H.J. Lu wrote:
> >        PR rtl-optimization/49154
> >        * ira-costs.c (setup_regno_cost_classes_by_mode): If there
> >        already is a matching slot in the hashtable, assign it to
> >        classes_ptr.
> >
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49354

Fixed.  Cross-builds apparently don't include -Werror, meh.
Tested by observing warning without, no warning with, committed
as obvious.  Thanks for keeping me honest.  I'm just going to
rant a little and say it's weird that we have to cast a "void *"
to another pointer type, but such is C++ or whatever we're
writing now.

	PR bootstrap/49354
	* ira-costs.c (setup_regno_cost_classes_by_mode): Add missing cast
	to last assignment.

Index: ira-costs.c
===================================================================
--- ira-costs.c	(revision 174868)
+++ ira-costs.c	(working copy)
@@ -300,7 +300,7 @@ setup_regno_cost_classes_by_mode (int re
 	  *slot = classes_ptr;
 	}
       else
-	classes_ptr = *slot;
+	classes_ptr = (cost_classes_t) *slot;
       cost_classes_mode_cache[mode] = (cost_classes_t) *slot;
     }
   regno_cost_classes[regno] = classes_ptr;

brgds, H-P