<p dir="ltr">Hi Hal,</p>
<p dir="ltr">On Jul 16, 2014 8:21 PM, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br>
><br>
> Hi Justin,<br>
><br>
> Thanks for working on this!<br>
><br>
> As Bill said, we need test cases. In addition, a few comments:</p>
<p dir="ltr">I have only one more test case to provide for this and I think it should be ready.</p>
<p dir="ltr">><br>
> +  const MCExpr *tocExpr = MCBinaryExpr::CreateAdd(MCSymbolRefExpr::Create(CurrentPos, OutContext),<br>
><br>
> +                                                  MCConstantExpr::Create(0x8000, OutContext),<br>
><br>
> +                                                  OutContext);<br>
><br>
> what is 0x8000? Please provide a comment.</p>
<p dir="ltr">I will provide a comment on this.  The GOT pointer points to the middle of the GOT which is 64k in total size.</p>
<p dir="ltr">><br>
>        BuildMI(FirstMBB, MBBI, dl, TII.get(PPC::MFLR), GlobalBaseReg);<br>
><br>
> +      if (PPCSubTarget->isTargetELF()) {<br>
><br>
> +        unsigned TempReg = RegInfo->createVirtualRegister(&PPC::GPRCRegClass);<br>
><br>
> +        BuildMI(FirstMBB, MBBI, dl, TII.get(PPC::GetGBRO), TempReg).addReg(GlobalBaseReg);<br>
><br>
> +        BuildMI(FirstMBB, MBBI, dl, TII.get(PPC::UpdateGBR)).addReg(GlobalBaseReg).addReg(TempReg);<br>
><br>
> +        MF->getInfo<PPCFunctionInfo>()->setUsesPICBase(true);<br>
><br>
> It is okay here for TempReg to be R0?</p>
<p dir="ltr">Since it is active for only two instructions r0 should be fine.</p>
<p dir="ltr">><br>
>          // automatically synthesizes these stubs.<br>
><br>
> -        OpFlags = PPCII::MO_DARWIN_STUB;<br>
><br>
> +        OpFlags = PPCII::MO_PLT_STUB;<br>
><br>
> is this going to break Darwin?</p>
<p dir="ltr">No. I overrode this constant to serve the same purpose in ELF as it serves in Darwin.</p>
<p dir="ltr">><br>
>        // unless we're building with the leopard linker or later, which<br>
><br>
>        // automatically synthesizes these stubs.<br>
><br>
> -      OpFlags = PPCII::MO_DARWIN_STUB;<br>
><br>
> +      OpFlags = PPCII::MO_PLT_STUB;<br>
><br>
> this too? Darwin breaking?</p>
<p dir="ltr">Again nope.</p>
<p dir="ltr">><br>
> -  explicit PPCFunctionInfo(MachineFunction &MF)<br>
> +  explicit PPCFunctionInfo(MachineFunction &MFC)<br>
> ...<br>
> +      MF(MFC),<br>
><br>
> This is unnecessary; because of the way that C++ name lookup works, MF(MF) will be fine here.</p>
<p dir="ltr">Gotcha. My c++fu is weak, didn't know this.</p>
<p dir="ltr">Thanks for the review. I will have another patch for you layer tonight or tomorrow.</p>
<p dir="ltr">- Justin</p>
<p dir="ltr">><br>
> Thanks again,<br>
> Hal<br>
><br>
> ----- Original Message -----<br>
> > From: "Justin Hibbits" <<a href="mailto:chmeeedalf@gmail.com">chmeeedalf@gmail.com</a>><br>
> > To: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > Sent: Friday, July 11, 2014 7:54:08 PM<br>
> > Subject: [PATCH] 32-bit PowerPC ELF Position Independent Code<br>
> ><br>
> > Hi,<br>
> ><br>
> > For the last few weeks I've been working off and on with ppc32 PIC<br>
> > for<br>
> > ELF/SysVR4, and now have something to show for it.  The patch is<br>
> > attached.  It passes all existing tests, and I haven't yet added new<br>
> > tests for this.  I also tested by compiling zlib on FreeBSD/PowerPC,<br>
> > and running some commands linked against it.<br>
> ><br>
> > Differences from GCC:<br>
> ><br>
> > * GCC sets aside r30 as the PIC register, this will use any available<br>
> >   register.<br>
> > * GNU AS writes a real difference to the function pre-word, whereas<br>
> > the<br>
> >   LLVM assembler writes 0, with a relocation.  This appears to be<br>
> >   irrelevant, as it works correctly in my test case.<br>
> ><br>
> > - Justin<br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
> ><br>
><br>
> --<br>
> Hal Finkel<br>
> Assistant Computational Scientist<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<br>
</p>