r228253 - [PowerPC] Revert workaround for TLS linker bug

[Hal Finkel]

----- 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.

 -Hal

> 
> Bill
> 
> > 
> > Bill
> > 
> > > 
> > > Details to follow.
> > > 
> > > Bill
> > 
> 
> 
> 

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