This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [mips] fix $gp restore bug
- From: Adam Nemet <anemet at caviumnetworks dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: Nathan Sidwell <nathan at codesourcery dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 14 Sep 2009 11:11:07 -0700
- Subject: Re: [mips] fix $gp restore bug
- References: <49870BD7.8050707@codesourcery.com> <87iqnswrzu.fsf@firetop.home> <4987F33C.1060408@codesourcery.com> <8763jrcmdo.fsf@firetop.home> <87r61fli49.fsf@firetop.home> <alpine.LFD.1.10.0903022352330.18052@ftp.linux-mips.org> <o7zlg3o17c.fsf@ropi.home> <alpine.LFD.1.10.0903031037310.18052@ftp.linux-mips.org> <18861.21372.902140.780295@ropi.home> <alpine.LFD.1.10.0903051153050.18052@ftp.linux-mips.org> <18864.222.258830.920265@ropi.home> <87sklrskee.fsf@firetop.home> <18864.56135.151151.775345@ropi.home> <87eiqrkd3e.fsf@firetop.home> <CAEXCH01QtgTIFprhMO000003d8@caexch01.caveonetworks.com> <871vmazjjr.fsf@firetop.home>
Richard Sandiford writes:
> You're right that cprestores ought to be deleted when
> !mips_chosen_to_use_gp_p && optimize, but we don't generally delete
> them when optimisation is disabled. So if we kept the original check,
> we would sometimes be left with loads from an uninitialised cprestore slot.
> As well as being untidy, I was worried that it might confuse valgrind-like
> analysis tools into thinking there was a bug in the program. I also
> though it was odd to introduce real loads before we had introduced
> real stores.
>
> This does show up as a failure in branch-8.c.
I see.
> However, I can see your point about it being confusing to have this one
> place where !mips_chosen_to_use_gp_p is not a reversible decision.
> And comments alone aren't really enough to compensate.
>
> I think the confusion would remain whatever we call the function,
> so what do you think about the alternative below? It instead has two
> state variables, one to decide whether the global pointer value needs
> to be initialised, and another to say whether pic_offset_table_rtx
> ought to be restored after being clobbered. We can then document
> more easily that the latter variable has a very limited lifetime,
> whereas the former applies right to the end of compilation.
Yes, I think it works much better having separate flags. Thanks!
Just another minor question. In the "big comment" you reshuffled the order of
the points. Previously they had been close to order in the pass pipeline.
Was this intentional? I think the old was probably easier to follow.
Adam