[llvm-commits] [PATCH] [MC] .pushsection / .popsection support

Nick Lewycky nlewycky at google.com
Wed Feb 9 15:38:56 PST 2011


On 9 February 2011 13:40, Joerg Sonnenberger <joerg at britannica.bec.de>wrote:

> On Wed, Feb 09, 2011 at 01:29:23PM -0800, Nick Lewycky wrote:
> > On 9 February 2011 12:28, Joerg Sonnenberger <joerg at britannica.bec.de
> >wrote:
> >
> > > Hi all,
> > > attached patch is the initial support for .pushsection / .popsection.
> > > To do this properly requires a change in the MCStreamer API. One
> > > approach is attached. I think a better idea would be to split the
> > > current SwitchSection() into two parts: the common code to manage
> > > CurSection / PrevSection and a virtual function ChangeSection that gets
> > > called if the new CurSection differs from the old one. There are a
> bunch
> > > of bugs with the handling of changing to the current section again.
> > >
> >
> > Why does this need a second SwitchSection method and why does it need to
> > take the previous as an argument? Why not just take the current one, then
> > switch to the new one?
>
> Because .popstack changes both the current and last section. More
> importantly, it can set the previous section to NULL, so it can't just
> be emulated with two calls to SwitchSection.
>

Right, I realize that it can't, but the MCStreamer API is supposed to mirror
the set of directives actually supported. SwitchSection maps to ".section"
but SwitchSection2 doesn't as there's no directive which set the current and
previous section to any strings you want. The MCStreamer implementations can
just modify CurSection and PrevSection directly, they don't need an API for
it.

Splitting the current SwitchSection would be cleaner and fix some issues
> as I mentioned.
>

Sure, feel free to refactor but please make them protected methods.
Currently CurSection and PrevSection are protected. I'd suggest replacing
those with the stacks and then get{Current,Previous}Section return the top
of the stack.

> Also, did you check that a .s which consists entirely of ".previous" gives
> a
> > nice error? GAS says:
> > x.s:1: Warning: .previous without corresponding .section; ignored
>
> I haven't touched .previous and don't plan to do it for this patch.
> It is an unrelated issue.
>

Good point!

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110209/60701347/attachment.html>


More information about the llvm-commits mailing list