diff mbox

[LRA] PR 61522 - Handle NULL targetm.spill_class

Message ID 539F174F.8060802@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov June 16, 2014, 4:11 p.m. UTC
On 2014-06-16, 11:05 AM, Ramana Radhakrishnan wrote:
> Hi,
>
>      This handles NULL targetm.spill_class in assign_by_spills. This
> showed up as a segfault during a build for arm-none-linux-gnueabi(hf).
>
> Fix pre-approved by richi on IRC , verified that bootstrap continues
> from where things broke further on a tree (that reverts 211600 which is
> the next breakage causing commit on arm-none-linux-gnueabihf)
>
> I'll apply this in about 45 minutes when I get back to my desk if no one
> objects.
>
>

Ops, I've already committed the following patch (sorry I've not read 
email before doing this):

I apologize for my mistake with the original patch.

2014-06-16  Vladimir Makarov  <vmakarov@redhat.com>

         PR rtl-optimization/61522
         * lra-assigns.c (assign_by_spills): Check null
         targetm.spill_class.

Comments

Ramana Radhakrishnan June 16, 2014, 4:14 p.m. UTC | #1
On Mon, Jun 16, 2014 at 5:11 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
> On 2014-06-16, 11:05 AM, Ramana Radhakrishnan wrote:
>>
>> Hi,
>>
>>      This handles NULL targetm.spill_class in assign_by_spills. This
>> showed up as a segfault during a build for arm-none-linux-gnueabi(hf).
>>
>> Fix pre-approved by richi on IRC , verified that bootstrap continues
>> from where things broke further on a tree (that reverts 211600 which is
>> the next breakage causing commit on arm-none-linux-gnueabihf)
>>
>> I'll apply this in about 45 minutes when I get back to my desk if no one
>> objects.
>>
>>
>
> Ops, I've already committed the following patch (sorry I've not read email
> before doing this):
>
> I apologize for my mistake with the original patch.
>
> 2014-06-16  Vladimir Makarov  <vmakarov@redhat.com>
>
>         PR rtl-optimization/61522
>         * lra-assigns.c (assign_by_spills): Check null
>         targetm.spill_class.
>
> Index: lra-assigns.c
> ===================================================================
> --- lra-assigns.c       (revision 211710)
> +++ lra-assigns.c       (working copy)
> @@ -1425,7 +1425,8 @@
>
>               enum reg_class rclass = lra_get_allocno_class (regno);
>               enum reg_class spill_class;
>
> -             if (lra_reg_info[regno].restore_regno < 0
> +             if (targetm.spill_class == NULL
> +                 || lra_reg_info[regno].restore_regno < 0
>                   || ! bitmap_bit_p (&lra_inheritance_pseudos, regno)
>                   || (spill_class
>                       = ((enum reg_class)

Any reason the check couldn't be like the earlier patch ?

i.e. else if (targetm.spill_class)
 {

....

}


Ramana

>
Vladimir Makarov June 16, 2014, 4:29 p.m. UTC | #2
On 2014-06-16, 12:14 PM, Ramana Radhakrishnan wrote:
> On Mon, Jun 16, 2014 at 5:11 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:

> Any reason the check couldn't be like the earlier patch ?
>
> i.e. else if (targetm.spill_class)
>   {
>
> ....
>
> }
>

I've just preferred to put most conditions in one place.  That is the 
only reason.  But if you want you can commit your variant.

I would have approved your patch, if I read your email before.  I was in 
hurry to fix it as the original patch broke most LRA targets (ones 
without the hook definition) and the mistake itself was embarrassing for me.
diff mbox

Patch

Index: lra-assigns.c
===================================================================
--- lra-assigns.c       (revision 211710)
+++ lra-assigns.c       (working copy)
@@ -1425,7 +1425,8 @@ 
               enum reg_class rclass = lra_get_allocno_class (regno);
               enum reg_class spill_class;

-             if (lra_reg_info[regno].restore_regno < 0
+             if (targetm.spill_class == NULL
+                 || lra_reg_info[regno].restore_regno < 0
                   || ! bitmap_bit_p (&lra_inheritance_pseudos, regno)
                   || (spill_class
                       = ((enum reg_class)