[llvm] r283086 - [PowerPC] Account for the ELFv2 function prologue during branch selection

[Eric Christopher via llvm-commits]

On Wed, Oct 26, 2016 at 3:52 PM Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
> > From: "Eric Christopher" <echristo at gmail.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>, llvm-commits at lists.llvm.org
> > Sent: Thursday, October 20, 2016 8:46:08 PM
> > Subject: Re: [llvm] r283086 - [PowerPC] Account for the ELFv2 function
> prologue during branch selection
> >
> >
> > Hrm. Would it make sense to have the code in EmitFunctionBodyStart
> > emit instructions into the first basic block instead? (Will that
> > work here?)
> >
>
> The instructions would need to be added before branch relaxation runs. We
> could do this with a separate pass that runs before branch relaxation. Not
> sure it is worth the complexity. Perhaps the next bug that comes up because
> of this I'll change my mind? ;)
>
>
Makes sense. Weirdly complicated at the moment, but I can see the
additional work because of branch relaxation too. :)

-eric


>  -Hal
>
> >
> > Just thinking out loud.
> >
> >
> > -eric
> >
> >
> > On Sun, Oct 2, 2016 at 9:15 PM Hal Finkel via llvm-commits <
> > llvm-commits at lists.llvm.org > wrote:
> >
> >
> > Author: hfinkel
> > Date: Sun Oct 2 23:06:44 2016
> > New Revision: 283086
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=283086&view=rev
> > Log:
> > [PowerPC] Account for the ELFv2 function prologue during branch
> > selection
> >
> > The PPC branch-selection pass, which performs branch relaxation,
> > needs to
> > account for the padding that might be introduced to satisfy block
> > alignment
> > requirements. We were assuming that the first block was at offset
> > zero (i.e.
> > had the alignment of the function itself), but under the ELFv2 ABI, a
> > global
> > entry function prologue is added to the first block, and it is a
> > two-instruction sequence (i.e. eight-bytes long). If the function has
> > 16-byte
> > alignment, the fact that the first block is eight bytes offset from
> > the start
> > of the function is relevant to calculating where padding will be
> > added in
> > between later blocks.
> >
> > Unfortunately, I don't have a small test case.
> >
> > Modified:
> > llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp
> > llvm/trunk/lib/Target/PowerPC/PPCBranchSelector.cpp
> >
> > Modified: llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp?rev=283086&r1=283085&r2=283086&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp (original)
> > +++ llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp Sun Oct 2
> > 23:06:44 2016
> > @@ -1199,6 +1199,9 @@ void PPCLinuxAsmPrinter::EmitFunctionBod
> > if (Subtarget->isELFv2ABI()
> > // Only do all that if the function uses r2 in the first place.
> > && !MF->getRegInfo().use_empty(PPC::X2)) {
> > + // Note: The logic here must be synchronized with the code in the
> > + // branch-selection pass which sets the offset of the first block
> > in the
> > + // function. This matters because it affects the alignment.
> > const PPCFunctionInfo *PPCFI = MF->getInfo<PPCFunctionInfo>();
> >
> > MCSymbol *GlobalEntryLabel = PPCFI->getGlobalEPSymbol();
> >
> > Modified: llvm/trunk/lib/Target/PowerPC/PPCBranchSelector.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCBranchSelector.cpp?rev=283086&r1=283085&r2=283086&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/PowerPC/PPCBranchSelector.cpp (original)
> > +++ llvm/trunk/lib/Target/PowerPC/PPCBranchSelector.cpp Sun Oct 2
> > 23:06:44 2016
> > @@ -19,8 +19,10 @@
> > #include "MCTargetDesc/PPCPredicates.h"
> > #include "PPCInstrBuilder.h"
> > #include "PPCInstrInfo.h"
> > +#include "PPCSubtarget.h"
> > #include "llvm/ADT/Statistic.h"
> > #include "llvm/CodeGen/MachineFunctionPass.h"
> > +#include "llvm/CodeGen/MachineRegisterInfo.h"
> > #include "llvm/Support/MathExtras.h"
> > #include "llvm/Target/TargetMachine.h"
> > #include "llvm/Target/TargetSubtargetInfo.h"
> > @@ -92,8 +94,19 @@ bool PPCBSel::runOnMachineFunction(Machi
> > return AlignAmt + OffsetToAlignment(Offset, AlignAmt);
> > };
> >
> > + // We need to be careful about the offset of the first block in the
> > function
> > + // because it might not have the function's alignment. This happens
> > because,
> > + // under the ELFv2 ABI, for functions which require a TOC pointer,
> > we add a
> > + // two-instruction sequence to the start of the function.
> > + // Note: This needs to be synchronized with the check in
> > + // PPCLinuxAsmPrinter::EmitFunctionBodyStart.
> > + unsigned InitialOffset = 0;
> > + if (Fn.getSubtarget<PPCSubtarget>().isELFv2ABI() &&
> > + !Fn.getRegInfo().use_empty(PPC::X2))
> > + InitialOffset = 8;
> > +
> > // Measure each MBB and compute a size for the entire function.
> > - unsigned FuncSize = 0;
> > + unsigned FuncSize = InitialOffset;
> > for (MachineFunction::iterator MFI = Fn.begin(), E = Fn.end(); MFI !=
> > E;
> > ++MFI) {
> > MachineBasicBlock *MBB = &*MFI;
> > @@ -240,7 +253,7 @@ bool PPCBSel::runOnMachineFunction(Machi
> > if (MadeChange) {
> > // If we're going to iterate again, make sure we've updated our
> > // padding-based contributions to the block sizes.
> > - unsigned Offset = 0;
> > + unsigned Offset = InitialOffset;
> > for (MachineFunction::iterator MFI = Fn.begin(), E = Fn.end(); MFI !=
> > E;
> > ++MFI) {
> > MachineBasicBlock *MBB = &*MFI;
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161027/5d99dadf/attachment-0001.html>