| Message ID | 539F07D0.3040609@arm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 16, 2014 at 04:05:52PM +0100, Ramana Radhakrishnan wrote: > 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. Please only commit the first hunk, not the second. That looks like incorrectly indenting something that has been correctly indented before. > 2014-06-16 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> > > PR rtl-optimization/61522 > * lra-assigns.c (assign_by_spills): Handle NULL targetm.spill_class. > diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c > index cea4c33..5de18e1 100644 > --- a/gcc/lra-assigns.c > +++ b/gcc/lra-assigns.c > @@ -1420,7 +1420,7 @@ assign_by_spills (void) > alternatives of insns containing the pseudo. */ > bitmap_set_bit (&changed_pseudo_bitmap, regno); > } > - else > + else if (targetm.spill_class) > { > enum reg_class rclass = lra_get_allocno_class (regno); > enum reg_class spill_class; > @@ -1438,12 +1438,12 @@ assign_by_spills (void) > if (hard_regno < 0) > regno_allocno_class_array[regno] = rclass; > else > - { > - setup_reg_classes > - (regno, spill_class, spill_class, spill_class); > - assign_hard_regno (hard_regno, regno); > - bitmap_set_bit (&changed_pseudo_bitmap, regno); > - } > + { > + setup_reg_classes > + (regno, spill_class, spill_class, spill_class); > + assign_hard_regno (hard_regno, regno); > + bitmap_set_bit (&changed_pseudo_bitmap, regno); > + } > } > } > } Jakub
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 >
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 --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c index cea4c33..5de18e1 100644 --- a/gcc/lra-assigns.c +++ b/gcc/lra-assigns.c @@ -1420,7 +1420,7 @@ assign_by_spills (void) alternatives of insns containing the pseudo. */ bitmap_set_bit (&changed_pseudo_bitmap, regno); } - else + else if (targetm.spill_class) { enum reg_class rclass = lra_get_allocno_class (regno); enum reg_class spill_class; @@ -1438,12 +1438,12 @@ assign_by_spills (void) if (hard_regno < 0) regno_allocno_class_array[regno] = rclass; else - { - setup_reg_classes - (regno, spill_class, spill_class, spill_class); - assign_hard_regno (hard_regno, regno); - bitmap_set_bit (&changed_pseudo_bitmap, regno); - } + { + setup_reg_classes + (regno, spill_class, spill_class, spill_class); + assign_hard_regno (hard_regno, regno); + bitmap_set_bit (&changed_pseudo_bitmap, regno); + } } } }
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. regards Ramana 2014-06-16 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> PR rtl-optimization/61522 * lra-assigns.c (assign_by_spills): Handle NULL targetm.spill_class.