[llvm] r217801 - Fix a lot of confusion around inserting nops on empty functions.
Eric Christopher
echristo at gmail.com
Mon Sep 15 12:42:13 PDT 2014
Thanks for doing this Rafael!
-eric
On Mon, Sep 15, 2014 at 11:32 AM, Rafael Espindola <
rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Mon Sep 15 13:32:58 2014
> New Revision: 217801
>
> URL: http://llvm.org/viewvc/llvm-project?rev=217801&view=rev
> Log:
> Fix a lot of confusion around inserting nops on empty functions.
>
> On MachO, and MachO only, we cannot have a truly empty function since that
> breaks the linker logic for atomizing the section.
>
> When we are emitting a frame pointer, the presence of an unreachable will
> create a cfi instruction pointing past the last instruction. This is
> perfectly
> fine. The FDE information encodes the pc range it applies to. If some tool
> cannot handle this, we should explicitly say which bug we are working
> around
> and only work around it when it is actually relevant (not for ELF for
> example).
>
> Given the unreachable we could omit the .cfi_def_cfa_register, but then
> again, we could also omit the entire function prologue if we wanted to.
>
> Removed:
> llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll
> llvm/trunk/test/CodeGen/SPARC/empty-functions.ll
> Modified:
> llvm/trunk/include/llvm/Target/TargetInstrInfo.h
> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
> llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp
> llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h
> llvm/trunk/test/CodeGen/X86/empty-functions.ll
>
> Modified: llvm/trunk/include/llvm/Target/TargetInstrInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Target/TargetInstrInfo.h (original)
> +++ llvm/trunk/include/llvm/Target/TargetInstrInfo.h Mon Sep 15 13:32:58
> 2014
> @@ -847,10 +847,8 @@ public:
> MachineBasicBlock::iterator MI) const;
>
>
> - /// getNoopForMachoTarget - Return the noop instruction to use for a
> noop.
> - virtual void getNoopForMachoTarget(MCInst &NopInst) const {
> - // Default to just using 'nop' string.
> - }
> + /// Return the noop instruction to use for a noop.
> + virtual void getNoopForMachoTarget(MCInst &NopInst) const;
>
>
> /// isPredicated - Returns true if the instruction is already
> predicated.
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=217801&r1=217800&r2=217801&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Mon Sep 15 13:32:58
> 2014
> @@ -731,12 +731,10 @@ void AsmPrinter::EmitFunctionBody() {
>
> // Print out code for the function.
> bool HasAnyRealCode = false;
> - const MachineInstr *LastMI = nullptr;
> for (auto &MBB : *MF) {
> // Print a label for the basic block.
> EmitBasicBlockStart(MBB);
> for (auto &MI : MBB) {
> - LastMI = &MI;
>
> // Print the assembly for the instruction.
> if (!MI.isPosition() && !MI.isImplicitDef() && !MI.isKill() &&
> @@ -797,24 +795,14 @@ void AsmPrinter::EmitFunctionBody() {
> EmitBasicBlockEnd(MBB);
> }
>
> - // If the last instruction was a prolog label, then we have a situation
> where
> - // we emitted a prolog but no function body. This results in the ending
> prolog
> - // label equaling the end of function label and an invalid "row" in the
> - // FDE. We need to emit a noop in this situation so that the FDE's rows
> are
> - // valid.
> - bool RequiresNoop = LastMI && LastMI->isCFIInstruction();
> -
> // If the function is empty and the object file uses
> .subsections_via_symbols,
> // then we need to emit *something* to the function body to prevent the
> // labels from collapsing together. Just emit a noop.
> - if ((MAI->hasSubsectionsViaSymbols() && !HasAnyRealCode) ||
> RequiresNoop) {
> + if ((MAI->hasSubsectionsViaSymbols() && !HasAnyRealCode)) {
> MCInst Noop;
> TM.getSubtargetImpl()->getInstrInfo()->getNoopForMachoTarget(Noop);
> - if (Noop.getOpcode()) {
> - OutStreamer.AddComment("avoids zero-length function");
> - OutStreamer.EmitInstruction(Noop, getSubtargetInfo());
> - } else // Target not mc-ized yet.
> - OutStreamer.EmitRawText(StringRef("\tnop\n"));
> + OutStreamer.AddComment("avoids zero-length function");
> + OutStreamer.EmitInstruction(Noop, getSubtargetInfo());
> }
>
> const Function *F = MF->getFunction();
>
> Modified: llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp Mon Sep 15 13:32:58 2014
> @@ -372,6 +372,10 @@ static const TargetRegisterClass *canFol
> return nullptr;
> }
>
> +void TargetInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
> + llvm_unreachable("Not a MachO target");
> +}
> +
> bool TargetInstrInfo::
> canFoldMemoryOperand(const MachineInstr *MI,
> const SmallVectorImpl<unsigned> &Ops) const {
>
> Modified: llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp (original)
> +++ llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp Mon Sep 15 13:32:58 2014
> @@ -37,11 +37,6 @@ SparcInstrInfo::SparcInstrInfo(SparcSubt
> RI(ST), Subtarget(ST) {
> }
>
> -/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
> -void SparcInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
> - NopInst.setOpcode(SP::NOP);
> -}
> -
> /// isLoadFromStackSlot - If the specified machine instruction is a direct
> /// load from a stack slot, return the virtual or physical register
> number of
> /// the destination along with the FrameIndex of the loaded stack slot.
> If
>
> Modified: llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h (original)
> +++ llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h Mon Sep 15 13:32:58 2014
> @@ -93,8 +93,6 @@ public:
> const TargetRegisterInfo *TRI) const override;
>
> unsigned getGlobalBaseReg(MachineFunction *MF) const;
> -
> - void getNoopForMachoTarget(MCInst &NopInst) const override;
> };
>
> }
>
> Removed: llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll?rev=217800&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll (original)
> +++ llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll (removed)
> @@ -1,13 +0,0 @@
> -; RUN: llc < %s -mtriple=powerpc-apple-darwin | FileCheck
> -check-prefix=CHECK-NO-FP %s
> -; RUN: llc < %s -mtriple=powerpc-apple-darwin -disable-fp-elim |
> FileCheck -check-prefix=CHECK-FP %s
> -; RUN: llc < %s -mtriple=powerpc-netbsd -disable-fp-elim | FileCheck
> -check-prefix=CHECK-FP %s
> -
> -define void @func() {
> -entry:
> - unreachable
> -}
> -; CHECK-NO-FP: _func:
> -; CHECK-NO-FP: nop
> -
> -; CHECK-FP: {{_?}}func:
> -; CHECK-FP: nop {{[;#]}} avoids zero-length function
>
> Removed: llvm/trunk/test/CodeGen/SPARC/empty-functions.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SPARC/empty-functions.ll?rev=217800&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/SPARC/empty-functions.ll (original)
> +++ llvm/trunk/test/CodeGen/SPARC/empty-functions.ll (removed)
> @@ -1,8 +0,0 @@
> -; RUN: llc < %s -mtriple=sparc-unknown-openbsd -disable-fp-elim |
> FileCheck -check-prefix=CHECK-FP-LABEL %s
> -
> -define void @func() {
> -entry:
> - unreachable
> -}
> -; CHECK-FP-LABEL: {{_?}}func:
> -; CHECK-FP-LABEL: nop {{[;!]}} avoids zero-length function
>
> Modified: llvm/trunk/test/CodeGen/X86/empty-functions.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/empty-functions.ll?rev=217801&r1=217800&r2=217801&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/empty-functions.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/empty-functions.ll Mon Sep 15 13:32:58 2014
> @@ -1,10 +1,14 @@
> ; RUN: llc < %s -mtriple=x86_64-apple-darwin | FileCheck
> -check-prefix=CHECK-NO-FP %s
> ; RUN: llc < %s -mtriple=x86_64-apple-darwin -disable-fp-elim | FileCheck
> -check-prefix=CHECK-FP %s
> +; RUN: llc < %s -mtriple=x86_64-linux-gnu | FileCheck
> -check-prefix=LINUX-NO-FP %s
> +; RUN: llc < %s -mtriple=x86_64-linux-gnu -disable-fp-elim | FileCheck
> -check-prefix=LINUX-FP %s
>
> define void @func() {
> entry:
> unreachable
> }
> +
> +; MachO cannot handle an empty function.
> ; CHECK-NO-FP: _func:
> ; CHECK-NO-FP-NEXT: .cfi_startproc
> ; CHECK-NO-FP: nop
> @@ -21,5 +25,30 @@ entry:
> ; CHECK-FP-NEXT: movq %rsp, %rbp
> ; CHECK-FP-NEXT: :
> ; CHECK-FP-NEXT: .cfi_def_cfa_register %rbp
> -; CHECK-FP-NEXT: nop
> ; CHECK-FP-NEXT: .cfi_endproc
> +
> +; An empty function is perfectly fine on ELF.
> +; LINUX-NO-FP: func:
> +; LINUX-NO-FP-NEXT: .cfi_startproc
> +; LINUX-NO-FP-NEXT: {{^}}#
> +; LINUX-NO-FP-NEXT: {{^}}.L{{.*}}:{{$}}
> +; LINUX-NO-FP-NEXT: .size func, .L{{.*}}-func
> +; LINUX-NO-FP-NEXT: .cfi_endproc
> +
> +; A cfi directive can point to the end of a function. It (and in fact the
> +; entire body) could be optimized out because of the unreachable, but we
> +; don't do it right now.
> +; LINUX-FP: func:
> +; LINUX-FP-NEXT: .cfi_startproc
> +; LINUX-FP-NEXT: {{^}}#
> +; LINUX-FP-NEXT: pushq %rbp
> +; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
> +; LINUX-FP-NEXT: .cfi_def_cfa_offset 16
> +; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
> +; LINUX-FP-NEXT: .cfi_offset %rbp, -16
> +; LINUX-FP-NEXT: movq %rsp, %rbp
> +; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
> +; LINUX-FP-NEXT: .cfi_def_cfa_register %rbp
> +; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
> +; LINUX-FP-NEXT: .size func, .Ltmp3-func
> +; LINUX-FP-NEXT: .cfi_endproc
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140915/f97e0a58/attachment.html>
More information about the llvm-commits
mailing list