[PATCH][PowerPC] Fix BlockAddress access on ppc64

[Hal Finkel]

----- Original Message -----
> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> To: "Anton Blanchard" <anton at samba.org>
> Cc: "Hal J. Finkel" <hfinkel at anl.gov>, "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Thursday, October 30, 2014 7:29:59 PM
> Subject: Re: [PATCH][PowerPC] Fix BlockAddress access on ppc64
> 
> Anton Blanchard <anton at samba.org> wrote on 30.10.2014 21:41:25:
> 
> > > > The attached patch fixes this by loading such values from the
> > > > TOC,
> > > > as is done for any other global symbol.
> > >
> > > Thanks! I verified this fixed the issue for me.
> >
> > Sorry, I spoke too soon:
> >
> > kernel/built-in.o:(.toc+0x0): undefined reference to `.Ltmp371608'
> >
> > .Ltmp35:                                # Block address taken
> > # BB#1:                                 # %__here
> >         addis 3, 2, .LC7 at toc@ha
> >         li 4, 512
> >         ld 3, .LC7 at toc@l(3)
> >         bl __local_bh_enable_ip
> >
> > ...
> >
> > .LC7:
> >         .tc .Ltmp3543[TC],.Ltmp3543
> 
> Huh.  It seems there is a pre-existing bug in code to write out the
> TOC that cannot handle temporary labels:
> 
>      for (MapVector<MCSymbol*, MCSymbol*>::iterator I = TOC.begin(),
>           E = TOC.end(); I != E; ++I) {
>        OutStreamer.EmitLabel(I->second);
>        MCSymbol *S =
>        OutContext.GetOrCreateSymbol(I->first->getName());
>        if (isPPC64)
>          TS.emitTCEntry(*S);
> 
> The GetOrCreateSymbol call will note that ".Ltmp35" is not a symbol
> (since it's a temporary label), so it will attempt to create a new
> symbol with that name.  This in turn will notice that the name
> ".Ltmp35" *is* already in use (as a temporary label), and thus
> refuse to create a symbol of the same name.  However, there is a
> fallback path that assumes that since this is a temporary name,
> it can be simply renamed, and does so ...
> 
> This is all particularly weird since I->first above already *is* a
> MCSymbol, and I cannot see any reason why we'd need to create
> *another* MCSymbol just to print it out.  Simply using I->first
> directly fixes the problem for me.
> 
> I've attached an updated version that implements that solution,
> and enhances the test to actually verify the correct label name.
> (See attached file: diff-llvm-ppc64-blockaddress)
> 
> Still OK for mainline?

Yep, still looks good :-) -- go right ahead.

Thanks again,
Hal

> 
> Bye,
> Ulrich

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory