[PATCH] 32-bit PowerPC ELF Position Independent Code

[Justin Hibbits]

Hi Hal,

Here's the latest patch with whitespace and line fixes.  Let me know if
there are any other issues, cosmetic or otherwise.  Some of the
whitespace I think looks a little weird, because I tried to follow
indenting rules to line up function arguments, as it appears llvm-style
is, and it came out kind of weird with outdents due to nested function
calls.

- Justin

On Fri, 18 Jul 2014 01:09:52 -0500
Hal Finkel <hfinkel at anl.gov> wrote:

> Justin,
> 
> Thanks, this looks good. You have a bunch of lines that are too long
> (80 characters is the limit). Now, parts of that code have
> long-standing violations of this, but let's not make it worse. Please
> wrap those lines that would go over 80 characters.
> 
>  -Hal
> 
> ----- Original Message -----
> > From: "Justin Hibbits" <chmeeedalf at gmail.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: llvm-commits at cs.uiuc.edu
> > Sent: Friday, July 18, 2014 12:31:05 AM
> > Subject: Re: [PATCH] 32-bit PowerPC ELF Position Independent Code
> > 
> > Hi Hal,
> > 
> > Thanks for looking at this.  Got another one for you now.
> > 
> > On Wed, 16 Jul 2014 22:20:59 -0500
> > Hal Finkel <hfinkel at anl.gov> wrote:
> > 
> > > Hi Justin,
> > > 
> > > Thanks for working on this!
> > > 
> > > As Bill said, we need test cases. In addition, a few comments:
> > > 
> > > +  const MCExpr *tocExpr =
> > > MCBinaryExpr::CreateAdd(MCSymbolRefExpr::Create(CurrentPos,
> > > OutContext),
> > > 
> > > +
> > > MCConstantExpr::Create(0x8000, OutContext),
> > > 
> > > +                                                  OutContext);
> > > 
> > > what is 0x8000? Please provide a comment.
> > > 
> > >        BuildMI(FirstMBB, MBBI, dl, TII.get(PPC::MFLR),
> > >        GlobalBaseReg);
> > > 
> > > +      if (PPCSubTarget->isTargetELF()) {
> > > 
> > > +        unsigned TempReg =
> > > RegInfo->createVirtualRegister(&PPC::GPRCRegClass);
> > > 
> > > +        BuildMI(FirstMBB, MBBI, dl, TII.get(PPC::GetGBRO),
> > > TempReg).addReg(GlobalBaseReg);
> > > 
> > > +        BuildMI(FirstMBB, MBBI, dl,
> > > TII.get(PPC::UpdateGBR)).addReg(GlobalBaseReg).addReg(TempReg);
> > > 
> > > +        MF->getInfo<PPCFunctionInfo>()->setUsesPICBase(true);
> > > 
> > > It is okay here for TempReg to be R0?
> > > 
> > >          // automatically synthesizes these stubs.
> > > 
> > > -        OpFlags = PPCII::MO_DARWIN_STUB;
> > > 
> > > +        OpFlags = PPCII::MO_PLT_STUB;
> > > 
> > > is this going to break Darwin?
> > > 
> > >        // unless we're building with the leopard linker or later,
> > > which
> > > 
> > >        // automatically synthesizes these stubs.
> > > 
> > > -      OpFlags = PPCII::MO_DARWIN_STUB;
> > > 
> > > +      OpFlags = PPCII::MO_PLT_STUB;
> > > 
> > > this too? Darwin breaking?
> > > 
> > > -  explicit PPCFunctionInfo(MachineFunction &MF)
> > > +  explicit PPCFunctionInfo(MachineFunction &MFC)
> > > ...
> > > +      MF(MFC),
> > > 
> > > This is unnecessary; because of the way that C++ name lookup
> > > works, MF(MF) will be fine here.
> > > 
> > > Thanks again,
> > > Hal
> > 
> > I think I've addressed all your comments in my latest patch.  Mind
> > giving it another look?
> > 
> > - Justin
> > 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ppc32pic_jul18.diff
Type: text/x-patch
Size: 35807 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140718/4009adca/attachment.bin>