r228253 - [PowerPC] Revert workaround for TLS linker bug

[Bill Schmidt]

On Fri, 2015-02-06 at 12:07 -0600, Hal Finkel wrote:
> ----- Original Message -----
> > From: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com>
> > To: "Chandler Carruth" <chandlerc at google.com>
> > Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>, "Hal Finkel" <hfinkel at anl.gov>
> > Sent: Thursday, February 5, 2015 7:19:21 PM
> > Subject: Re: r228253 - [PowerPC] Revert workaround for TLS linker bug
> > 
> > On Thu, 2015-02-05 at 19:13 -0600, Bill Schmidt wrote:
> > > On Thu, 2015-02-05 at 18:52 -0600, Bill Schmidt wrote:
> > > > On Wed, 2015-02-04 at 18:30 -0800, Chandler Carruth wrote:
> > > > > 
> > > > > On Wed, Feb 4, 2015 at 5:12 PM, Bill Schmidt
> > > > > <wschmidt at linux.vnet.ibm.com> wrote:
> > > > >         Author: wschmidt
> > > > >         Date: Wed Feb  4 19:12:24 2015
> > > > >         New Revision: 228253
> > > > >         
> > > > >         URL:
> > > > >         http://llvm.org/viewvc/llvm-project?rev=228253&view=rev
> > > > >         Log:
> > > > >         [PowerPC] Revert workaround for TLS linker bug
> > > > >         
> > > > >         In r227480, Ulrich Weigand introduced a workaround for
> > > > >         a
> > > > >         linker
> > > > >         optimization bug that can create mis-optimized code for
> > > > >         accesses to
> > > > >         general-dynamic or local-dynamic TLS variables.  The
> > > > >         linker
> > > > >         optimization bug only occurred for Clang/LLVM because
> > > > >         of some
> > > > >         inefficient code being generated for these TLS
> > > > >         accesses.  I
> > > > >         have
> > > > >         recently corrected LLVM to produce the efficient code
> > > > >         sequence
> > > > >         expected by the linkers, so this workaround is no
> > > > >         longer
> > > > >         needed.
> > > > >         Therefore this patch reverts r227480.
> > > > >         
> > > > >         I've tested that the previous bootstrap failure no
> > > > >         longer
> > > > >         occurs with
> > > > >         the workaround reverted.
> > > > > 
> > > > > This is awesome Bill, thanks again for tackling all of this.
> > > > 
> > > > Well, unfortunately it isn't going to stick. :(
> > > > 
> > > > A bootstrap with -O3 still fails, and there is a further,
> > > > different, and
> > > > more horrible linker optimization error to blame.  I'm going to
> > > > re-disable the linker optimizations for now.  I'm unwilling to
> > > > handcuff
> > > > the compiler as much as would be required to let the linker
> > > > optimization
> > > > operate in its present state.
> > > 
> > > Correction: Still too soon to blame the linker here.  My eyes were
> > > playing tricks on me.  Will take a fresh look at the cause of the
> > > bootstrap error in the morning.
> > 
> > And as soon as I press Send, I see it.  I was looking at the wrong
> > destructor this time.
> > 
> > Essentially what happens is this:  I have a new pass that forces a
> > register copy of the __tls_get_call parameter into GPR3.  In this
> > case
> > RA is apparently unable to coalesce the copy away, which leaves the
> > addi
> > targeting a non-GPR3 register again.  So the known linker
> > optimization
> > bug kicks in.
> > 
> > I will have to require def-use chains for the new pass so that I can
> > perform direct operand replacement instead of using register copies.
> > Will have a look at this tomorrow.
> 
> I think there is an easy way to do this: Create a register class that contains only PPC::X3. Schedule your pass to run prior to register allocation, and call MRI.constrainRegClass on the virtual registers feeding the relevant instructions so that they'll need to use that register.

Ah, that indeed sounds like a much simpler approach.  I will give that a
shot.  Thank you!

Bill

> 
>  -Hal
> 
> > 
> > Bill
> > 
> > > 
> > > Bill
> > > 
> > > > 
> > > > Details to follow.
> > > > 
> > > > Bill
> > > 
> > 
> > 
> > 
>